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

Try a full screen mode appear animation #9910

Merged
merged 1 commit into from
Sep 20, 2018
Merged

Conversation

kjellr
Copy link
Contributor

@kjellr kjellr commented Sep 14, 2018

This PR tries out a slight animation to full screen mode when it's loaded:

  • The body does a quick 0-100% fade animation.
  • The toolbar slides in from the top.

This is subtle (and due to frame rates, it cannot be observed in the GIF — so please give this PR a spin before weighing in) but helps the transition feel little more smooth.

A couple notes:

  • This appears every time the body.is-fullscreen-mode loads, so if you activate full screen, navigate away from the page, and come back, it'll animate again on the initial editor load. This actually seems just fine to me in practice.
  • The fade in animation is 0.3s long, which is a little slower than I'd usually do. But since this fades in the whole page content, it has a larger chance of seeming jarring. I think the extra 0.1-0.2s helps make this feel snappy but still calm.

GIFs

Old (no animation):

old

New:

new-animation

This adds a slight animation to full screen mode when its loaded:

- The body does a quick 0-100% fade animation.
- The toolbar slides in from the top.
@kjellr kjellr added the Needs Design Feedback Needs general design feedback. label Sep 14, 2018
@kjellr kjellr self-assigned this Sep 14, 2018
@jasmussen
Copy link
Contributor

I dig it!

Assuming no, but just because it has to be asked, there's no way we can _scale the window upwards and leftwards, right? If possible that would be sweeter than the fade.

However the fade seems cool in place if it.

Is there a way we can test performance on this? Anyone have an old clanker around to test this on? Opacity is usually a relatively expensive operation depending on how it's implemented — i.e. creating a giant white layer on top and fading that out is actually cheaper than it is to change the opacity of text, if I recall correctly. Not sure how we can verify this, it seems cool to me, but it would be nice to know that it's not horrendous on old machines. CC: @shaunandrews @MichaelArestad for CSS-wizardry thoughts here.

@kjellr
Copy link
Contributor Author

kjellr commented Sep 17, 2018

Assuming no, but just because it has to be asked, there's no way we can _scale the window upwards and leftwards, right? If possible that would be sweeter than the fade.

So, after doing a little more digging, we could do something like this:

new-animation

(I slowed that down a little bit for GIF frame rate purposes)

It's a little rough, but you can check that out in the try/full-screen-animation branch. Some caveats:

  • I expanded the scope of the animations to include the admin bar and the wp sidebar (so they can slide in/out). This is great because they animate when you enter and exit full screen mode. The major downside is that they animate on each page load. 😄 That obviously doesn't work for us. So we'd need to find a way to work around that.
  • Also, I don't love setting global transition values on important selectors like #adminmenumain, #wpadminbar, #wpcontent, and #wpfooter. This is done so we can animate in + out, but it also creates the issue above.
  • I think I can tweak the body animation to use position instead of margin, but #wpcontent, and #wpfooter will need their margins animated. My recollection is that animating margins is expensive performance-wise. 😕

Whichever direction we go in, I'd love to get some performance thoughts from Shaun + Michael.

@MichaelArestad
Copy link
Contributor

Assuming no, but just because it has to be asked, there's no way we can _scale the window upwards and leftwards, right? If possible that would be sweeter than the fade.

Definitely doable, but hacky. For example, we wouldn't want to actually resize the editor at all. Luckily, with all that white space we could sort of fake it by:

  • using transform to move the editor itself left and up a bit
  • using transform to move the sidebar out of the window
  • using transform to move the wp-admin bar up out of the window

The real problem lies in any UI within the editor that is full width. For example, the toolbar. If we resize that via animation, it's extremely expensive and will be much slower on less powerful devices. I think there are perhaps clever ways around this as well, but generally I would lean against this approach as it is quite complex and prone to be broken down the road.

@MichaelArestad
Copy link
Contributor

Some things to note: box-model changes (width, margin, padding, border) are all very expensive. Transform and opacity are cheap and smooth.

@kjellr
Copy link
Contributor Author

kjellr commented Sep 18, 2018

Quick update:

After a chat with @shaunandrews, I also explored animating the size of the white container (instead of animating the sidebar and adminbar off screen). This seems nice too:

new-animation

... but I can't see any way to do it without animating the width of the editor, which is surely not performant. So it's not likely to be a workable solution.

@jasmussen
Copy link
Contributor

Thanks so much for the exploration.

Just out of an abundance of performance caution, perhaps we should go back to the initial fade for now, and let this be an opportunity on the table for continued refinement.

@kjellr
Copy link
Contributor Author

kjellr commented Sep 19, 2018

Just out of an abundance of performance caution, perhaps we should go back to the initial fade for now, and let this be an opportunity on the table for continued refinement.

That sounds reasonable to me — the original animation uses just an opacity fade + the toolbar slide in, so both be relatively performant (especially in comparison to the other options).

Would you mind giving the PR a final review in that case? When you do, be sure to leave fullscreen mode on, and exit to the "All Posts" screen. Then open up a few more posts, and observe that there's a quick fade in when they open automatically into full screen mode. I think this is fine (and I wasn't actually seeing it in all cases? 🤔), but I could use another gut check on that too.

Thanks, all!

Copy link
Contributor

@jasmussen jasmussen left a comment

Choose a reason for hiding this comment

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

Ship it!

@kjellr kjellr added this to the 4.0 milestone Sep 20, 2018
@kjellr kjellr merged commit 0480601 into master Sep 20, 2018
@kjellr kjellr deleted the try/full-screen-animation branch September 20, 2018 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Feedback Needs general design feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants