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

Document Settings: Fix document title hover and select animations #25719

Merged
merged 9 commits into from Oct 2, 2020

Conversation

@jeyip
Copy link
Contributor

@jeyip jeyip commented Sep 29, 2020

Description

The hover and select animation for document title can be improved in a few ways as identified by the design team:

  • The template part name appears to overflow the topbar, and doesn't fade-in
  • The label colors and boldness don't match what has been specified in mocks
  • The appearance of the secondary label clips the bottom border of the topbar

We are using these updates as an opportunity to refactor the technical implementation to improve animation performance.

Ideally, the animations would look like this.

document-info-animation

Here's the same animation slowed down to help see the transitions:

document-info-animation-slow

How has this been tested?

  1. Spin up a local WordPress instance. From the project root:
    a. Execute npx wp-env start in the terminal
    b. Execute npm run dev in the terminal
  2. Open the WordPress UI at http://localhost:8888/wp-admin and sign in with the username admin and the password password.
  3. Enable site editing in the local dev environment
    a. Click on Appearance < Themes in the sidebar and activate the seedlet blocks theme
    b. Click on Gutenberg < Experiments in the sidebar and check the Full Site Editing Feature
  4. Navigate to the site editor
  5. Hover over any template part and note the document title hover animation
  6. Select any template part and note the document title select animation
  7. Verify that these match the ideal animations provided by the design team
  8. Ensure that the solution respects responsive behavior (when the browser viewport is shrunk or enlarged)

Screenshots

Before

document-info-animate-bug

After

2020-10-01 16 44 54

Types of changes

Fixes #25545

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.
@github-actions
Copy link

@github-actions github-actions bot commented Sep 29, 2020

Size Change: +6.33 kB (0%)

Total Size: 1.18 MB

Filename Size Change
build/a11y/index.js 1.14 kB -1 B
build/api-fetch/index.js 3.35 kB +11 B (0%)
build/block-directory/index.js 8.55 kB +23 B (0%)
build/block-editor/index.js 129 kB +350 B (0%)
build/block-editor/style-rtl.css 11 kB -124 B (1%)
build/block-editor/style.css 11 kB -125 B (1%)
build/block-library/editor-rtl.css 8.6 kB +45 B (0%)
build/block-library/editor.css 8.6 kB +44 B (0%)
build/block-library/index.js 135 kB +142 B (0%)
build/block-library/style-rtl.css 7.65 kB +56 B (0%)
build/block-library/style.css 7.65 kB +61 B (0%)
build/block-serialization-default-parser/index.js 1.78 kB +1 B
build/blocks/index.js 47.5 kB +61 B (0%)
build/components/index.js 169 kB +1.03 kB (0%)
build/components/style-rtl.css 15.4 kB -82 B (0%)
build/components/style.css 15.4 kB -81 B (0%)
build/compose/index.js 9.42 kB +1 B
build/core-data/index.js 12 kB +26 B (0%)
build/data-controls/index.js 685 B -584 B (85%) 🏆
build/data/index.js 8.6 kB +176 B (2%)
build/edit-navigation/index.js 10.7 kB +231 B (2%)
build/edit-post/index.js 306 kB +3 B (0%)
build/edit-post/style-rtl.css 6.25 kB +10 B (0%)
build/edit-post/style.css 6.24 kB +10 B (0%)
build/edit-site/index.js 20.5 kB +720 B (3%)
build/edit-site/style-rtl.css 3.83 kB +529 B (13%) ⚠️
build/edit-site/style.css 3.83 kB +529 B (13%) ⚠️
build/edit-widgets/index.js 21.1 kB +3.54 kB (16%) ⚠️
build/edit-widgets/style-rtl.css 3 kB +202 B (6%) 🔍
build/edit-widgets/style.css 3 kB +203 B (6%) 🔍
build/editor/index.js 45.5 kB -42 B (0%)
build/editor/style-rtl.css 3.83 kB +26 B (0%)
build/editor/style.css 3.82 kB +24 B (0%)
build/element/index.js 4.44 kB -7 B (0%)
build/escape-html/index.js 734 B +1 B
build/format-library/index.js 7.49 kB -5 B (0%)
build/i18n/index.js 3.54 kB -2 B (0%)
build/is-shallow-equal/index.js 710 B +1 B
build/media-utils/index.js 5.12 kB -1 B
build/plugins/index.js 2.44 kB -1 B
build/priority-queue/index.js 790 B +1 B
build/redux-routine/index.js 2.85 kB -1 B
build/rich-text/index.js 13 kB -673 B (5%)
build/server-side-render/index.js 2.6 kB -3 B (0%)
build/url/index.js 4.06 kB +2 B (0%)
build/warning/index.js 1.13 kB -1 B
ℹ️ View Unchanged
Filename Size Change
build/annotations/index.js 3.52 kB 0 B
build/autop/index.js 2.72 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-library/theme-rtl.css 741 B 0 B
build/block-library/theme.css 741 B 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/date/index.js 31.9 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 4.42 kB 0 B
build/edit-navigation/style-rtl.css 868 B 0 B
build/edit-navigation/style.css 871 B 0 B
build/editor/editor-styles-rtl.css 492 B 0 B
build/editor/editor-styles.css 493 B 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 1.74 kB 0 B
build/html-entities/index.js 621 B 0 B
build/keyboard-shortcuts/index.js 2.39 kB 0 B
build/keycodes/index.js 1.85 kB 0 B
build/list-reusable-blocks/index.js 3.02 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/notices/index.js 1.69 kB 0 B
build/nux/index.js 3.27 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/primitives/index.js 1.34 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.24 kB 0 B
build/viewport/index.js 1.74 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@jeyip jeyip force-pushed the fix/document-title-hover-and-select-animations branch from 3131529 to 13e65ef Sep 30, 2020
@jeyip
Copy link
Contributor Author

@jeyip jeyip commented Sep 30, 2020

Note: There's curious behavior (that exists on master). When a template part or template part child is already selected, the document secondary title will still change when a different template part is hovered. It seems odd to me, but I was wondering what other folks thought of this.

2020-09-30 09 41 13

@jeyip jeyip changed the title Document Settings: Fix document title hover and select animations [IN PROGRESS] Document Settings: Fix document title hover and select animations Sep 30, 2020
@noahtallen
Copy link
Member

@noahtallen noahtallen commented Sep 30, 2020

the document secondary title will still change when a different template part is hovered. It seems odd to me, but I was wondering what other folks thought of this.

I think this would be good to think about in a follow-up. I don't know what we expect to happen design-wise

@jeyip jeyip marked this pull request as ready for review Sep 30, 2020
@jeyip jeyip requested a review from youknowriad as a code owner Sep 30, 2020
@jeyip jeyip requested review from noahtallen and Addison-Stavlo Sep 30, 2020
@noahtallen
Copy link
Member

@noahtallen noahtallen commented Sep 30, 2020

Ok, wow, this is way better than it was before! Nice job.

Two thoughts:

  1. Should the main text be bold when it's the only item in the header?
  2. Should there be a slight transition when the text changes to bold? I think the designs might include that
@noahtallen noahtallen requested a review from shaunandrews Sep 30, 2020
@noahtallen
Copy link
Member

@noahtallen noahtallen commented Oct 1, 2020

Another note is that we should include the reduce-motion mixin

Copy link
Contributor

@Addison-Stavlo Addison-Stavlo left a comment

Oops, I didn't realize this is based off the hover-block-tracking PR and not master. I was a bit confused when I saw hover working. 🤣

Other than what Noah noted, I am finding some new issues with the titles now wrapping:

Screen Shot 2020-10-01 at 11 17 01 AM

instead of the previous:

Screen Shot 2020-10-01 at 11 16 13 AM

@jeyip
Copy link
Contributor Author

@jeyip jeyip commented Oct 1, 2020

1. Should the main text be bold when it's the only item in the header?
I can see this going either way. I think it makes sense to make the document title more discoverable, even if it's the only item. If, however, we're trying to draw attention to the hover and select interactions, I think the current behavior makes more sense (although it can be a bit jarring).

Thoughts @shaunandrews?

See Addison's comment below . Adding in the boldness now.

2. Should there be a slight transition when the text changes to bold? I think the designs might include that
I should've left a note for this, but I excluded this transition because font-weight changes cause reflow. The transition isn't the smoothest. I'm not sure if there's a way around this, but I can try exploring this further if it's important to us.

2020-10-01 15 41 58

Side Note:
We'll probably want a strategy for longer template part titles (15+ characters). At a certain point, the title will butt up against either side of the topbar and affect the responsiveness of the topbar buttons. Maybe something like text ellipses?

Screen Shot 2020-10-01 at 3 49 19 PM

@jeyip
Copy link
Contributor Author

@jeyip jeyip commented Oct 1, 2020

Another note is that we should include the reduce-motion mixin

I love that this exists. 🚀 Great callout Noah.

@Addison-Stavlo
Copy link
Contributor

@Addison-Stavlo Addison-Stavlo commented Oct 1, 2020

  1. Should the main text be bold when it's the only item in the header?

From the mock ups shared (#25348 (comment)) it looks like it should be bold when it is the only item.

I should've left a note for this, but I excluded this transition because font-weight changes cause reflow.

Ah, interesting!

At a certain point, the title will butt up against either side of the topbar and affect the responsiveness of the topbar buttons.

Yeah, that top bar breaks easily when it overflows. Hopefully getting rid of that double dropdown in the near future will help. But we may still need to consider what to do about excessively long names. Maybe a max-width for the element and either let it overflow or cut off the overflow? 🤔 Either way, thats outside of this PR.

@jeyip
Copy link
Contributor Author

@jeyip jeyip commented Oct 1, 2020

From the mock ups shared (#25348 (comment)) it looks like it should be bold when it is the only item.

🤦‍♂️ Y'all are totally right. Don't know why I didn't see that. I'm switching it now!

@noahtallen
Copy link
Member

@noahtallen noahtallen commented Oct 2, 2020

I'll make a follow up issue for the width of characters. Interesting about the font weight transition! Avoiding it makes sense to me.

Copy link
Member

@noahtallen noahtallen left a comment

This is working very well for me. Let's ship it! :shipit:

@noahtallen
Copy link
Member

@noahtallen noahtallen commented Oct 2, 2020

I guess we should rebase on master first though :)

@Addison-Stavlo
Copy link
Contributor

@Addison-Stavlo Addison-Stavlo commented Oct 2, 2020

should we base these off master / merge to that? I see this is still opened against the hover PR which is still in ???s (hopefully g2g soon)

@noahtallen
Copy link
Member

@noahtallen noahtallen commented Oct 2, 2020

I'll make a follow up issue for the width of characters

Here: #25783

@jeyip jeyip requested a review from TimothyBJacobs as a code owner Oct 2, 2020
@jeyip jeyip changed the base branch from try/hovered-block-tracking to master Oct 2, 2020
@jeyip jeyip merged commit 7493226 into master Oct 2, 2020
15 checks passed
15 checks passed
@github-actions
Cancel Previous Runs
Details
@github-actions
Check Check
Details
@github-actions
build
Details
@github-actions
Admin - 1
Details
@github-actions
Compare performance with master
Details
@github-actions
pull-request-automation
Details
@github-actions
test (gutenberg-editor-gallery)
Details
@github-actions
test (gutenberg-editor-gallery)
Details
@github-actions
JavaScript
Details
@github-actions
Admin - 2
Details
@github-actions
Admin - 3
Details
@github-actions
Mobile
Details
@github-actions
Admin - 4
Details
@jeyip jeyip deleted the fix/document-title-hover-and-select-animations branch Oct 2, 2020
@github-actions github-actions bot added this to the Gutenberg 9.2 milestone Oct 2, 2020
kevin940726 added a commit that referenced this pull request Oct 6, 2020
…5719)

* Refactor hover and select animations

* Remove redundant styling

* Add slide down animation

* Remove unnecessary label class

* Implement transition instead of animation

* Add nowrap for secondary title

* Honor reduced motion setting in user OS

* Implement reduce motion mixin

* Add bold font weight to lone template title
kevin940726 added a commit that referenced this pull request Oct 6, 2020
…5719)

* Refactor hover and select animations

* Remove redundant styling

* Add slide down animation

* Remove unnecessary label class

* Implement transition instead of animation

* Add nowrap for secondary title

* Honor reduced motion setting in user OS

* Implement reduce motion mixin

* Add bold font weight to lone template title
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants