Skip to content

Comments

fix!: Allow to inherit from View to create custom classes#1067

Closed
susnux wants to merge 1 commit intomainfrom
fix/allow-custom-view
Closed

fix!: Allow to inherit from View to create custom classes#1067
susnux wants to merge 1 commit intomainfrom
fix/allow-custom-view

Conversation

@susnux
Copy link
Contributor

@susnux susnux commented Sep 3, 2024

Use case:
You want to scope logic and state into the view rather than somewhere in a module.

Problems:
Currently there are two problem that prevent creating a child-class of view:

  1. The view class did not implement the methods as methods but getters, this forces also child classes to use getters leading to really ugly code.
  2. The constructor required getContents to be part of the ViewData, but for child classes this makes no sense, as you probably want to overwrite this with a member function instead. Fixed by checking if new View was called or a new subclass.

Use case:
You want to scope logic and state into the view rather than somewhere in a module.

Problems:
Currently there are two problem that prevent creating a child-class of view:
1. The view class did not implement the methods as methods but getters,
   this forces also child classes to use getters leading to really ugly code.
2. The constructor required `getContents` to be part of the `ViewData`,
   but for child classes this makes no sense, as you probably want to overwrite this
   with a member function instead. Fixed by checking if `new View` was called or a new subclass.

Signed-off-by: Ferdinand Thiessen <[email protected]>
@susnux susnux added type: enhancement 🚀 New feature or request status: needs info ❓ Further information is requested 3. to review 3️⃣ Waiting for reviews labels Sep 3, 2024
@susnux susnux requested review from Pytal, ShGKme and skjnldsv September 3, 2024 22:15
@codecov
Copy link

codecov bot commented Sep 3, 2024

Bundle Report

Changes will increase total bundle size by 1.45kB ⬆️

Bundle name Size Change
@nextcloud/files-esm 118.08kB 722 bytes ⬆️
@nextcloud/files-esm-cjs 119.47kB 724 bytes ⬆️

@susnux
Copy link
Contributor Author

susnux commented Sep 3, 2024

This option is slightly breaking, but only for the files app as it need to check hasEmptyView before applying it as view.emptyView will now always be a function.

I think just extending View for custom views is more natural than just working with an data object (looses a bit of validation, but that is the risk of the implementor). But I am fine with any solution, and I have to admit the alternative in the PR below is simpler.


ℹ️ See alternative PR here

@skjnldsv
Copy link
Contributor

skjnldsv commented Sep 4, 2024

This option is slightly breaking, but only for the files app as it need to check hasEmptyView before applying it as view.emptyView will now always be a function.

Why not keep it as getter?

@susnux
Copy link
Contributor Author

susnux commented Sep 4, 2024

Why not keep it as getter?

Because you can not overwrite it in a custom class, it always then has to be a getter not a method.
Meaning this would be not be allowed:

class MyView extend View {

    public emptyView(div: HTMLDivElement) {
        // do something
    }
}

Instead you would need to do:

class MyView extend View {

    public get emptyView() {
        return this.handleEmptyView.bind(this)
    }

    private handleEmptyView(div: HTMLDivElement) {
        // ...
    }
}

@susnux
Copy link
Contributor Author

susnux commented Sep 4, 2024

(Thats the reason I also offer the alternative: #1068 )

@skjnldsv
Copy link
Contributor

skjnldsv commented Sep 5, 2024

Why would you need to overwrite it?
you can already customize the emptyView function when you create the View instance?

@susnux
Copy link
Contributor Author

susnux commented Sep 5, 2024

Why would you need to overwrite it?

Because e.g. I want to have a local scope, meaning the emptyView should not just be a loose function but bound to my class, so that I can access (private) class properties.
Meaning to be able to store view related state (e.g. the Vue instance of the empty view) inside of the View class instead being forced to store in in module scope.

@skjnldsv skjnldsv added 2. developing 2️⃣ Work in progress and removed 3. to review 3️⃣ Waiting for reviews labels Dec 11, 2025
@skjnldsv skjnldsv marked this pull request as draft December 11, 2025 08:11
@susnux susnux closed this in #1417 Dec 11, 2025
@susnux susnux deleted the fix/allow-custom-view branch December 21, 2025 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. developing 2️⃣ Work in progress status: needs info ❓ Further information is requested type: enhancement 🚀 New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants