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.

Sign up for GitHub

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

Jump to bottom

Dashboard: DashboardGrid - don't animate if reduced-motion set #75540

Merged
merged 1 commit into from Nov 29, 2023
Merged

Dashboard: DashboardGrid - don't animate if reduced-motion set #75540

merged 1 commit into from Nov 29, 2023

Conversation

dnwe
Copy link
Contributor

@dnwe dnwe commented Sep 27, 2023 •
edited

What is this feature?

Obey the prefers-reduced-motion CSS media feature and only enable the react-grid movement animations if the user hasn't set any preference for reduced motion

Why do we need this feature?

Accessibility support to avoid vestibular motion triggers as well as better support for headless browser image capture.

Which issue(s) does this PR fix?:

Contributes-to: #74603 (provides a workaround to disable the animation)

Special notes for your reviewer:

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@dnwe dnwe requested a review from a team as a code owner September 27, 2023 13:04
@dnwe dnwe requested review from eledobleefe and JoaoSilvaGrafana and removed request for a team September 27, 2023 13:04
@CLAassistant
Copy link

CLAassistant commented Sep 27, 2023 •
edited

CLA assistant check
All committers have signed the CLA.

@grafana-pr-automation grafana-pr-automation bot added area/frontend pr/external This PR is from external contributor labels Sep 27, 2023
@dnwe dnwe mentioned this pull request Sep 27, 2023
Open
@github-actions GitHub Actions
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 2 weeks if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot added the stale Issue with no recent activity label Nov 20, 2023
@dnwe
Copy link
Contributor Author

dnwe commented Nov 20, 2023

@tonypowa is there anyone available to triage this?

@github-actions github-actions bot removed the stale Issue with no recent activity label Nov 21, 2023
@tonypowa
Copy link
Contributor

routing PR to the dashboard squad
see #74603

@axelavargas axelavargas added this to the 10.3.x milestone Nov 28, 2023
@axelavargas axelavargas changed the title DashboardGrid: don't animate if reduced-motion set Dashboard: DashboardGrid - don't animate if reduced-motion set Nov 28, 2023
@axelavargas axelavargas added no-backport Skip backport of PR and removed no-backport Skip backport of PR triage/pending labels Nov 28, 2023
axelavargas
axelavargas approved these changes Nov 28, 2023
View reviewed changes
Copy link
Member

@axelavargas axelavargas left a comment

Choose a reason for hiding this comment

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

Hey, @dnwe 👋🏾 , thank you for contributing to Grafana :), this indeed is a good workaround for the original issue and also provides better accessibility.

The code looks good to me :), and testing locally also works as expected

@axelavargas
Copy link
Member

P.S. Is it possible for you to update your branch with the latest main branch?

@dnwe
DashboardGrid: don't animate if reduced-motion set
e723e71 
Obey the prefers-reduced-motion [1] CSS media feature and only enable the
react-grid movement animations if the user hasn't set any preference for
reduced motion

--
[1] https://developer.mozilla.org/en-US/docs/Web/CSS/@media/prefers-reduced-motion

Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>
@dnwe
Copy link
Contributor Author

dnwe commented Nov 28, 2023

@axelavargas thanks for the review! Branch re-based and pushed as requested

@axelavargas
Copy link
Member

Hey @dnwe 👋🏾 , can you re-sign the CLA? 🙈 for a strange reason now the CI is not reporting that status and it's required for the merge. Thank you for your patience with this one 🙏🏾 .

@dnwe
Copy link
Contributor Author

dnwe commented Nov 29, 2023

Hmm, cla-assistant definitely reports it as signed for me when I click back through to https://cla-assistant.io/grafana/grafana?pullRequest=75540 (and it still shows in the comment #75540 (comment) above) — not sure why the license/cla status check isn't posted/up-to-date

@dnwe

This comment was marked as outdated.

Sign in to view
@dnwe
Copy link
Contributor Author

dnwe commented Nov 29, 2023 •
edited

Aha, I googled a bit and found I could open https://cla-assistant.io/check/grafana/grafana?pullRequest=75540 to re-trigger the status check — it's showing green now

@axelavargas
Copy link
Member

Oh, nice finding! Thank you 🥳

@axelavargas axelavargas merged commit f41cf40 into grafana:main Nov 29, 2023
11 checks passed
@dnwe dnwe deleted the support-reduced-motion branch November 29, 2023 11:12
@dnwe
Copy link
Contributor Author

dnwe commented Nov 29, 2023

Thanks again for the review and testing

gabor pushed a commit that referenced this pull request Nov 29, 2023
@dnwe @gabor
Dashboard: DashboardGrid - don't animate if reduced-motion set (#75540)
da105c6
@aangelisc aangelisc modified the milestones: 10.3.x, 10.2.3 Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@axelavargas axelavargas axelavargas approved these changes

@nmarrs nmarrs Awaiting requested review from nmarrs

Assignees
No one assigned
Labels
add to changelog area/dashboard area/frontend no-backport Skip backport of PR pr/external This PR is from external contributor
Projects
Dashboards
Archived in project
Milestone
10.2.3
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@dnwe @CLAassistant @tonypowa @axelavargas @aangelisc