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

fix(v2): add support for prefers-reduced-motion in hideable sidebar #3739

Merged
merged 3 commits into from
Nov 16, 2020

Conversation

lex111
Copy link
Contributor

@lex111 lex111 commented Nov 13, 2020

Motivation

See #3615 (comment)

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Enable the reduce motion option and try hide doc sidebar.

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)

@lex111 lex111 requested a review from slorber as a code owner November 13, 2020 13:35
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Nov 13, 2020
@netlify
Copy link

netlify bot commented Nov 13, 2020

Deploy preview for docusaurus-2 ready!

Built with commit a9cc96c

https://deploy-preview-3739--docusaurus-2.netlify.app

@slorber slorber added the pr: bug fix This PR fixes a bug in a past release. label Nov 16, 2020
@slorber
Copy link
Collaborator

slorber commented Nov 16, 2020

LGTM, can confirm it fixes a bug with reduced motions.

Simek, can you confirm this fixed your issue? Do you have reduced motions enabled?

@slorber slorber merged commit 153d2d4 into master Nov 16, 2020
@Simek
Copy link
Contributor

Simek commented Nov 16, 2020

@slorber It fixes an issue for me too. According to the docs prefers-reduced-motion property depends on the system settings:

Interesting is that, according to MDN the user preferences should only affect Firefox.

@slorber
Copy link
Collaborator

slorber commented Nov 17, 2020

tested locally and can confirm the macOS system pref does impact chrome behavior

@lex111 lex111 deleted the lex111/hide-sidebar-reduce-motion branch November 17, 2020 15:01
@lex111 lex111 added this to the v2.0.0-alpha.67 milestone Nov 17, 2020
@Simek
Copy link
Contributor

Simek commented Nov 19, 2020

@lex111 @slorber After this change (or other CSS related changes, maybe even Infima bump) the following code is being added to the styles:

@media (prefers-reduced-motion: reduce) {
  * {
    transition: none !important
  }
}

witch breaks the animations on the React Native website:
anims

@lex111
Copy link
Contributor Author

lex111 commented Nov 19, 2020

Yep, probably we should have indicated about this in the release notes :/

This is one of the negative aspects of having such an opportunity.

Try use the !important rule to get around this behavior.

@Simek
Copy link
Contributor

Simek commented Nov 19, 2020

@lex111

https://www.w3.org/WAI/WCAG21/Techniques/css/C39.html:

This technique relates to Success Criterion 2.3.3: Animation from Interactions (Sufficient).

Some users experience distraction or nausea from animated content. For example, if scrolling a page causes elements to move (other than the essential movement associated with scrolling) it can trigger vestibular disorders. [...] A typical example is 'parallax scrolling', where backgrounds move at different rates. The movement due to scrolling the page is essential (and under the users control), but additional movement triggered by the scrolling can also trigger vestibular symptoms.

I think that static, in-place logo animation do not apply to the case for this property, the animation is not bound to the scrolling.

Also if you want to stick to the rule it only should apply only to the Docusaurus components, no the custom pages created by users.

@slorber
Copy link
Collaborator

slorber commented Nov 19, 2020

Hi @lex111

I think we may need to revert this change and apply it more sparingly.

We may prefer to avoid affecting the animations that Docusaurus users put on their pages , and only scope this reduced anim logic to infima classes only

Josh Comeau recommends opting-in for animations when user has no preference instead of opting out when he wants to remove motion here: https://joshwcomeau.com/react/prefers-reduced-motion/

Also his mediaquery seems more reliable that the current one (despite not recommended)

@media (prefers-reduced-motion: reduce) {
  * {
    animation-duration: 0.01ms !important;
    animation-iteration-count: 1 !important;
    transition-duration: 0.01ms !important;
    scroll-behavior: auto !important;
  }
}

maybe we should open a different issue because it's not really related to this PR, unfortunately as Infima is not open-sourced yet we can't discuss it there

@Simek
Copy link
Contributor

Simek commented Nov 19, 2020

@lex111 Also the implementation in Infima is incorrect because it do not cover the CSS Keyframes animation usage in conjunction with animation-name property, which leads to animation working fine in the V2 website header:
docu

@lex111
Copy link
Contributor Author

lex111 commented Nov 19, 2020 via email

@slorber
Copy link
Collaborator

slorber commented Nov 19, 2020

@lex111 will probably make a small bugfix release soon because the init template has a bug (#3784), and I'd like to have a stable release before merging i18n

Maybe can you revert this change for the bugfix release, and figure out a better way to apply it in an upcoming release?

@lex111
Copy link
Contributor Author

lex111 commented Nov 19, 2020 via email

@Simek
Copy link
Contributor

Simek commented Nov 19, 2020

@lex111 Will reverting Infima to alpha.16 or alpha.15 resolve the issue or were there more changes in those versions, which you don't want not to lose?

@lex111
Copy link
Contributor Author

lex111 commented Nov 21, 2020

Reduced interface support will be removed in Infima version .17 (the corresponding PR already merged). Let's postpone the adoption of this feature until we figure out the best way to implement it. @slorber will release new Docusaurus version on monday the next week, I guess.

@Simek until then, could you please build the RN site using .68v and confirm that it are rendered properly as before? The thing is, in the latest version of Docusaurus we have enabled the advanced minification of CSS bundle, and I want to know that in site like React Native that uses a lot of custom CSS code, nothing is broken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: bug fix This PR fixes a bug in a past release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants