-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RFC] Viewer 4.0 API #2395
Comments
API proposalRegistering handlers
Mime handling
Viewer// Init and get the viewer in Modal
export const getViewer = (viewer: any): Viewer => {}
// Create a new Viewer instance in the given element
export const createViewer = (el: HTMLElement, file: File): void => {}
type ViewerOptions = {
loadMore: Promise<File[]>
onPrev: () => void
onNext: () => void
onClose: () => void
canLoop: boolean
}
type Viewer = {
open: (nodes: File[], options?: ViewerOptions, handlerId?: string) => Promise<any>
openFolder: (folder: Folder, file: File, options?: ViewerOptions) => Promise<any>
compare: (node1: File, node2: File, handlerId?: string) => Promise<any>
} type Handler = {
id: string
displayName: string
group: string
enabled: (nodes: Node[]): boolean
render: (el: HTMLElement): void = {}
open: (file: File, visible = true, static = false) => Promise<any>
toggle: (visible: boolean): void => {}
}
Removed features ?
|
Please feel free to ask questions, discuss things I could have missed or whatever you see. Q&A
Relevant issues:
|
An alternative for the API that I discussed briefly with @ShGKme when talking about Vue3¹, instead of just rendering to a HTML element passed we could make use of custom elements (web components). You could have the element constructor as a getter on the handler like: interface IHandler {
id: string
displayName: string
group: string // what is this for?
enabled: (nodes: Node[]): boolean
// could also call it "element" or other...
readonly component: HTMLElement // this would be a getter for the custom component constructor
} Benefits are: We can pass props to custom elements and we can have events from the element, this allows communication between the rendered element and the viewer, so e.g. programmatically close the viewer, communicate loading state etc. ¹ because currently passing a Vue component is required, but this does not work with different Vues. So this might also be an interesting approach to discuss. |
🤩 such a good idea! Is there any pitfall we should be aware of? Reactivity? Css with shadow-root?... something else ? |
In addition to the render method, should we also have a destroy-like method on the handler to be able to cleanup once component is no longer active? The web components suggestion also sounds like a great idea but I don't have any knowledge around that to properly comment on it. |
If we go for web components, you can already define your own handlers when yur component is rendered and when it's destroyed with |
I played with Web Components in a similar problem just yesterday (always wanted to apply this approach in Nextcloud).
We have classic settings dialog in Talk.
I needed a public API in Talk to register new settings sections. But not like a classic child component, so there is no need to share Vue library instance. So, I tried to register custom settings section like Viewers but via Web Components. Benefits:
Problems:
Notes:
Some examples of different Vue 2 and Vue 3 based custom elements: |
Amazing feedback @ShGKme , thanks! For me the blockers would be:
The |
We can still define custom element manually (see example).
Creating a vanilla JS custom element that render the vue2 component is the only way to create Vue 2 based custom element. Anyway, the problem persists only if the host app is Vue 2. If Viewer 3.0 is on Vue 3 - there is no problem. |
Just wanted to note. Technically, Web Component's props are strings (like HTML attributes). But when it is used in Vue, Vue sets them as properties, not attributes, so we can path any objects to custom element props like with Vue components. |
I think even with shadow DOM you could include the server styles, like described here :)
There is e.g.: https://github.com/vuejs/vue-web-component-wrapper
Thats why I said "prop" not "attribute ;) |
That is an issue. If we wanna keep a compatibility with vanilla or even react, we need to make sure the behaviour is consistent. If only Vue web components are able to handle non-string props, but the other not, that is a big issue. Or did I misunderstood? |
Yes, styling isolation is amazing. That's why we scope our style almost everywhere here. |
No it is only about the "rendering" part not about the components.
But exactly as with every HTMLElement you can also pass things as properties, this is the same as doing:
So if you use web components in vue, Vue automatically passes props as properties (using the component instance). const myElement = new MyElement()
myElement.foo = 3
document.body.appendChild(myElement) For me this was quite helpful: https://open-wc.org/guides/knowledge/attributes-and-properties/ |
(We do one way scoping but this also is causing issues regularly)
Yes I agree, but as this would be new (while I really see a lot of use cases) we could of course provide a base class that is handling all this styling. This would make creating components very easy :) |
I'll have to think about this. From experience with designing APIs around here for a while now, I tend to want to avoid writing as much custom shortcuts for devs as possible. This always came back biting us after a bit. |
Looking at the Right now in files browser navigation will change the background while the viewer remains open and unchanged. If we push history entries during navigation that could maybe replace the |
I think we should be more elegant with this, you're right. RFC addition:
|
Let's discuss and plan the new Viewer handling.
cc @juliushaertl @susnux @R0Wi @ariselseng @artonge @max-nextcloud
Challenges
With the new Files app and the new Files/Folder/Node API, we finally have a proper and official way of dealing with files
It's time to re-draft the Viewer and clean the various issues we encountered over the last years
Requirements
Viewer
Handlers
Apps can register their own views, called "Handlers"
A handler provide the following
❓ Is this still the right approach?
true
/false
)The text was updated successfully, but these errors were encountered: