Skip to content
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

Modal component init #212

Merged
merged 2 commits into from
Feb 19, 2019
Merged

Modal component init #212

merged 2 commits into from
Feb 19, 2019

Conversation

skjnldsv
Copy link
Contributor

@skjnldsv skjnldsv commented Jan 23, 2019

Closes #210

Okay, so I did not wanted to implement anything more because I felt that it was the app's problem to handle this:

  • transitions: the modal can be use without pagination
  • closing: the app should manage if the component exists or not and if it's visible

Details:

  • I emit an event for each action possible (next, prev and close) and forward the event data to the parent. Do whatever you want with it :)
  • Just put whatever html/component you want inside this one, the slot will manage the rest.

Questions / issues:

  1. Should we rely on the modal component for the show/hide zoom animation? We could do that with the beforeMount/beforeDestroy stuff as well :)
  2. We NEED to find a solution for the translation
  3. the app-content have a z-index of 1000 so it means that the modal cannot be above the header (because zindex gets blocked by any lower parent) We need to fix it (yeaaah, let's touch zindexes again!! :O )
    SOLUTION: you cannot have the modal inside the #app-content element
  4. The animation doesn't work?
    SOLUTION: use v-if and not v-show for the modal
<Modal :has-previous="true" :has-next="false" :out-transition="true" @close="close"
	@previous="previous" @next="next">
	<span>YOUR MODAL CONTENT</span>
</Modal>

@skjnldsv skjnldsv added 2. developing Work in progress component Component discussion and/or suggestion labels Jan 23, 2019
@skjnldsv skjnldsv added this to the next milestone Jan 23, 2019
@skjnldsv skjnldsv self-assigned this Jan 23, 2019
@skjnldsv skjnldsv added the feature: modal Related to the modal component label Jan 23, 2019
@ChristophWurst
Copy link
Contributor

  1. show/hide zoom animation?

What's that?

2\. We NEED to find a solution for the translation

Yes.

@skjnldsv
Copy link
Contributor Author

What's that?

Small animation with a fadein+zoomin on show and the reverse on out :)

@ChristophWurst
Copy link
Contributor

Do it!

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv
Copy link
Contributor Author

skjnldsv commented Feb 19, 2019

kazam

@ChristophWurst @juliushaertl thoughts? :)
@jancborchardt

@jancborchardt
Copy link
Contributor

Regarding the animation, the question is what the context is:

  • If the context is "coming from outside", like the "About" window of firstrunwizard, the animation @juliushaertl chose for that is perfect. That is, a very quick centered fade collapse from outside.
  • If there’s no real context, the animation should be a centered fade zoom in from inside. Kind of like now, but it needs to be way faster and in the gif it looks like it moves up a bit first which is strange.
  • If the context is opening an image, the thumbnail of that file in the list/grid should morph to being the preview.

@skjnldsv
Copy link
Contributor Author

  • the thumbnail of that file in the list/grid should morph to being the preview.

I have absolutely no idea if I can do that 🤔

👍 for the rest! ;)

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv
Copy link
Contributor Author

skjnldsv commented Feb 19, 2019

Done!
@juliushaertl @ChristophWurst @nextcloud/vue please review! :)

Out In
kazam kazam

(Ignore the arrow staying, this is a quick vue example I did :p )

@skjnldsv skjnldsv added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Feb 19, 2019
Copy link
Contributor

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff design-wise! 👍

@skjnldsv skjnldsv merged commit 0e0f7bb into master Feb 19, 2019
@skjnldsv skjnldsv deleted the modal-component branch February 19, 2019 15:13
@juliushaertl
Copy link
Contributor

If the context is opening an image, the thumbnail of that file in the list/grid should morph to being the preview.

I doubt that the effort for such a implementation is worth it as the animation should be smooth and fast so a quick fade in + zoom is fine imo.

@juliushaertl juliushaertl mentioned this pull request Apr 15, 2019
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews component Component discussion and/or suggestion feature: modal Related to the modal component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dialog component
4 participants