Open skjnldsv opened 3 months ago
@nextcloud/viewer
library will be createdenabled
prop, which you'll be able to use to define the conditions in which your component can render the file properly. You can decide to keep checking the mime only if you want, but you'll also gain more flexibility.
This will also remove any fancy hacks with the mime aliases we were doing prior.// 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 => {}
}
open
is used when you already have a list and want to open it. It will not fetch any additional data unless you reach then end of your list and you provided a loadMore
method, in which case it will be called and the returned Files appended to the listopenFolder
is a bit closer to the old Viewer. It allow to just provide a folder, the Viewer will fetch the content and build up the list.
group
group
property, only the handler enabled
compare
is quite straightforward, most interactive features will be disabled by setting static
on open. Theoritecally you would be able to open any handler. :question: I hesitated to add a canCompare
boolean, but in the end, it depend on the dev. If you want to open to images side to side, you do you ! :shrug: enableSidebar
doesn't seem to be used anywhere, are we actually still using it?Please feel free to ask questions, discuss things I could have missed or whatever you see.
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). This way you have a framework agnostic way of registering components, it works with vanilla JS and also pretty easy with Vue (defineCustomElement).
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.
This also means no need to have render
, open
, toggle
methods on the handler, this are just props on the element like is-visible
, file
, etc. And one could have events like loaded
, close
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.
This also means no need to have
render
,open
,toggle
methods on the handler, this are just props on the element likeis-visible
,file
, etc. And one could have events likeloaded
,close
etc.
:star_struck: such a good idea! Is there any pitfall we should be aware of? Reactivity? Css with shadow-root?... something else ? Also: has anyone here already experimented with it at Nextcloud? I'd like Viewer to not be yet another experimentation place :see_no_evil:
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.
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?
If we go for web components, you can already define your own handlers when yur component is rendered and when it's destroyed with connectedCallback
and disconnectedCallback
:)
https://developer.mozilla.org/en-US/docs/Web/API/Web_components/Using_custom_elements#custom_element_lifecycle_callbacks
I played with Web Components in a similar problem just yesterday (always wanted to apply this approach in Nextcloud).
Benefits:
defineCustomElement
in upcoming Vue 3.5
Problems:
vue-devtools
doesn't see both Vue 2 and Vue 3 based custom elements
vue-devtools
can see
Notes:
Vue3.defineCustomElement
only supports disabling shadow DOM since last week in 3.5-beta.1Some examples of different Vue 2 and Vue 3 based custom elements:
Amazing feedback @ShGKme , thanks!
For me the blockers would be:
The vue-devtools
issue is annoying, but not the worst issue I've sen. We can work without it for vue2.
Viewer 3.0 would be vue3 anyway
- That mean we'll have to wait for 3.5 final.
We can still define custom element manually (see example). defineCustomElement
just makes it much simpler.
- Else maybe vue2 can hack its way around by creating a vanilla js custom element that render the vue2 component?
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.
We can pass props to custom elements
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.
disabling shadow DOM
I think even with shadow DOM you could include the server styles, like described here :) I personally really prefer the shadow DOM, the global styles are super fragile as apps often mess with them, especially as long as we do not properly use 2-way scoping (e.g. CSS modules).
Vue2 not being properly supported.
There is e.g.: https://github.com/vuejs/vue-web-component-wrapper (Also I really hope we quickly move away from Vue2, there are the first RCE :pensive: )
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.
Thats why I said "prop" not "attribute ;)
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.
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?
I think even with shadow DOM you could include the server styles, like described here :) I personally really prefer the shadow DOM, the global styles are super fragile as apps often mess with them, especially as long as we do not properly use 2-way scoping (e.g. CSS modules).
Yes, styling isolation is amazing. That's why we scope our style almost everywhere here.
But the theming engine is what I'm most worried about. We have things automatically set on root and body.
We CANNOT rely on Devs implementing their sharowroot adoptedStyleSheets
properly to work with viewer. They should only care about the props that are given and their implementation should seamlessly integrate into it.
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?
No it is only about the "rendering" part not about the components. Meaning if you have a element you can do:
<my-element foo="3" />
this is passing foo
as an attribute, the type will always be of type "string" (or boolean when done like <my-component foo />
).
But exactly as with every HTMLElement you can also pass things as properties, this is the same as doing:
myElement.bar = 3
So if you use web components in vue, Vue automatically passes props as properties (using the component instance). Which would be the same as doing this with vanilla JS:
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/
Yes, styling isolation is amazing. That's why we scope our style almost everywhere here.
(We do one way scoping but this also is causing issues regularly)
But the theming engine is what I'm most worried about. We have things automatically set on root and body. We CANNOT rely on Devs implementing their sharowroot
adoptedStyleSheets
properly to work with viewer. They should only care about the props that are given and their implementation should seamlessly integrate into it.
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 :)
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 onPrev
and onNext
callbacks in the API I wonder if it would be possible to take browser navigation into account.
Right now in files browser navigation will change the background while the viewer remains open and unchanged. For me it would be more intuitive if each viewed file had an entry in the browser history. When navigating back from the first file opened the viewer would close and display what was shown before.
If we push history entries during navigation that could maybe replace the onPrev
and onNext
callbacks - with listeners for the PopStateEvent. But frankly ... i don't really know what they are used for right now.
I think we should be more elegant with this, you're right.
init
and inject our own history handlerinit
history state and restore the previous history handler
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
true
/false
)