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

Conversation

Projects
None yet
3 participants
@kjellr
Contributor

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

Try a full screen mode appear 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 self-assigned this Sep 14, 2018

@kjellr kjellr requested a review from jasmussen Sep 14, 2018

@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Sep 17, 2018

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.

Contributor

jasmussen commented Sep 17, 2018

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

This comment has been minimized.

Show comment
Hide comment
@kjellr

kjellr Sep 17, 2018

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.

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.

Contributor

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

This comment has been minimized.

Show comment
Hide comment
@MichaelArestad

MichaelArestad 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.

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 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.

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

This comment has been minimized.

Show comment
Hide comment
@MichaelArestad

MichaelArestad Sep 17, 2018

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

MichaelArestad commented Sep 17, 2018

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

@kjellr

This comment has been minimized.

Show comment
Hide comment
@kjellr

kjellr Sep 18, 2018

Contributor

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.

Contributor

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

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Sep 19, 2018

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.

Contributor

jasmussen commented Sep 19, 2018

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

This comment has been minimized.

Show comment
Hide comment
@kjellr

kjellr Sep 19, 2018

Contributor

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!

Contributor

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!

@jasmussen

Ship it!

@kjellr kjellr added this to the 4.0 milestone Sep 20, 2018

@kjellr kjellr merged commit 0480601 into master Sep 20, 2018

2 checks passed

codecov/project 49.04% remains the same compared to 546b744
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@kjellr kjellr deleted the try/full-screen-animation branch Sep 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment