-
Notifications
You must be signed in to change notification settings - Fork 116
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
Animate annotation dialog #161
Animate annotation dialog #161
Conversation
@media #{$tablet} { | ||
width: 450px; | ||
animation: show-dialog-tablet; | ||
animation-duration: 0.2s; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note the duplicated animation duration and fill-mode.
These are nuked when applying the media query
…k/box-content-preview into Animate-annotation-dialog
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JustinHoldstock is there any kind of animation we can do when it's a plain highlight dialog? I think that's just distinguished by an added class on the usual mobile dialog class.
Ah this looks so cool!!! 👍 Recorded a gif I loved it so much :D
This made me think about how we're going to properly display dialogs on tablets. Right now it does cover some content when it slides out. We can run it by zach when i'm back :)
@@ -1,9 +1,45 @@ | |||
$tablet: "(min-width: 768px)"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is 768 a standard table size? Or is just applicable to our situation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the roughly the break point for an iPhone 6s (but still not as wide as a tablet in portrait), when rotated to landscape mode. I figured that anything larger than that would be considered "not fullscreen-able"
animation: show-dialog-tablet; | ||
animation-duration: .2s; | ||
animation-fill-mode: forwards; | ||
width: 450px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to do a % here because tablet sizes vary so much?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll talk to Zach about this one. We may want to go a fixed size for design purposes
…ew into Animate-annotation-dialog
…k/box-content-preview into Animate-annotation-dialog
Because we cover the top bar with all annotations, I decided to do the same with the plain highlight dialog (consistency). Otherwise, I'll have to rejigger the a ton of the CSS on the top bar. Demo: |
This PR is for a quick animation of the Annotation Dialog. Previous work to separate dialog system UI from functionality is in another branch.
Next up, investigate Highlight annotations, and discuss refactor of annotation dialog system.