jellyfin / jellyfin-vue

A modern web client for Jellyfin based on Vue
https://jellyfin.org
GNU General Public License v3.0
1.26k stars 226 forks source link

Add a modal/dialog system #135

Closed heyhippari closed 3 years ago

bbogdanov commented 3 years ago

@MrTimscampi Do you have an idea what this one should look like?

Do you see it like a single dialog that will be called with a configuration? Like:

{
   type: 'alert',
   payload: 'Some alert message'
   onClose: () => { }
}

// 

{
   type: 'confirm',
   payload: 'Are you sure you would like to change this? '
   onConfirm: () => {}
   onCancel: () => {}
}

//

{
   type: 'form',
   payload: {
      control: {
          name: 'username',
          label: 'Username'
          placeholder: 'e.g. John Doe'
          rules: [...]
         onChange: () => {}
      }
      ...
   }
   onSumbit: () => {}
   onCancel: () => {}
}
camc314 commented 3 years ago

It goes a little bit deeper than that see here. It's to managed components like the metadata editor, the identify dialog screen and other components that are overplayed on the screen. MrTim can probably provide a bit more info.

heyhippari commented 3 years ago

@bbogdanov Basically, we have multiple features that depend on modals, as well a need for some generic modals. Off the top of my head, we'd need:

I'm probably forgetting some.

Some modals are fairly simple (Just need to send a title, message and such), others are more complex and will likely depend on a BaseItemDto object or something else coming from the API.

It's still something that I sort of struggle designing, because I'm not sure what the best solution is.

In theory, simply putting the modals on the layout would be fine, I think, with something like v-show to control visibility. But I'm not positive if there's a better way for this or not (I've look at portal-vue and a few other things, but nothing quite strikes me as a great idea).

bbogdanov commented 3 years ago

The generic ones can be addressed in the layout and added to the DOM via v-if(reducing memory usage).

v-dialog uses overlay so vuetify can guarantee the modal will be always on top of everything when it opens.

These that are unique I believe have to be declared in the component that needs it.

In the other hand we have to agree that having more than one modal open at a time is not a good user experience so we might also adjust the code to follow this as well.

Are we aiming a11y support?

ferferga commented 3 years ago

@bbogdanov Everything we can do towards better accessibility is good, so I would say yes. We have terrible accessibility in the classic client and it's certainly something that has been overlooked.

I suppose that a11y will not only help us for accessibility to users but I guess it would also be helpful when we start designing a TV layout, as TVs require buttons to be correctly labeled for instance.

bbogdanov commented 3 years ago

Then we will have to address focus trap as well.

heyhippari commented 3 years ago

This can likely be closed now, since we've settled on a way of doing this.