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

Snackbar: Fix insufficient color contrast on hover #49682

Merged
merged 2 commits into from
Apr 14, 2023
Merged

Snackbar: Fix insufficient color contrast on hover #49682

merged 2 commits into from
Apr 14, 2023

Conversation

MahendraBishnoi29
Copy link
Contributor

@MahendraBishnoi29 MahendraBishnoi29 commented Apr 10, 2023

What?

this PR fixes the insufficient color contrast ratio on hover in the Snackbar component

Why?

#47273

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Apr 10, 2023
@github-actions
Copy link

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @MahendraBishnoi29! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@t-hamano
Copy link
Contributor

LGTM!

I think it is working as expected. It has not affected the existing focus style.

d7c72281828b9e392670acfc138eea57.mp4

One GitHub Actions is failing because a workflow was introduced in #49443 to check for CHANGELOG updates for the components package.

Could you add the following Bug Fix section under the Enhancements section in the Unreleased section?

### Enhancements

-   `DropZone`: Smooth animation ([#49517](https://github.com/WordPress/gutenberg/pull/49517)).
-   `Navigator`: Add `skipFocus` property in `NavigateOptions`. ([#49350](https://github.com/WordPress/gutenberg/pull/49350)).

### Bug Fix

-   `Snackbar`: Fix insufficient color contrast on hover ([#49682](https://github.com/WordPress/gutenberg/pull/49682)).

@t-hamano t-hamano added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Components /packages/components CSS Styling Related to editor and front end styles, CSS-specific issues. labels Apr 11, 2023
@MahendraBishnoi29
Copy link
Contributor Author

MahendraBishnoi29 commented Apr 12, 2023

LGTM!

I think it is working as expected. It has not affected the existing focus style.

One GitHub Actions is failing because a workflow was introduced in #49443 to check for CHANGELOG updates for the components package.

Could you add the following Bug Fix section under the Enhancements section in the Unreleased section?

### Enhancements

-   `DropZone`: Smooth animation ([#49517](https://github.com/WordPress/gutenberg/pull/49517)).
-   `Navigator`: Add `skipFocus` property in `NavigateOptions`. ([#49350](https://github.com/WordPress/gutenberg/pull/49350)).

### Bug Fix

-   `Snackbar`: Fix insufficient color contrast on hover ([#49682](https://github.com/WordPress/gutenberg/pull/49682)).

I didn't understand @t-hamano I mean like could you please submit a code review?

@t-hamano
Copy link
Contributor

t-hamano commented Apr 12, 2023

@MahendraBishnoi29

Sorry for the lack of explanation.

We would like to have one markdown file updated to correct an error in GitHub Action.

Here is the path to the file: ./packages/components/CHANGELOG.md

For this file, update it with what is described in this comment, commit, and push. The GitHub action will then be executed again, but the error should be resolved.

@MahendraBishnoi29
Copy link
Contributor Author

MahendraBishnoi29 commented Apr 12, 2023

@MahendraBishnoi29

Sorry for the lack of explanation.

We would like to have one markdown file updated to correct an error in GitHub Action.

Here is the path to the file: ./packages/components/CHANGELOG.md

For this file, update it with what is described in this comment, commit, and push. The GitHub action will then be executed again, but the error should be resolved.

here right?
Screenshot 2023-04-12 182227

@t-hamano
Copy link
Contributor

@MahendraBishnoi29

It is in the "Unreleased" section; you need to add a heading as well since we don't have "Bug Fix" section yet.

changelog

@MahendraBishnoi29
Copy link
Contributor Author

@MahendraBishnoi29

It is in the "Unreleased" section; you need to add a heading as well since we don't have "Bug Fix" section yet.

changelog

hey @t-hamano I actually made changes using the direct GitHub site instead of cloning into vscode so for pushing more commits I've to create a separate PR. so I think the reviewer has to add those commits to this PR or I've to make a new PR.

@t-hamano
Copy link
Contributor

@MahendraBishnoi29

I have added a commit updating CHANGELOG for your pull request. I would like to merge it once all GitHub Actions have passed. Thanks for your contribution 👍

@t-hamano
Copy link
Contributor

@chad1008

This PR was sent from a forked repository, with changes made to the component package files. Because the first PR didn't update the CHANGELOG, the workflow check added in #49443 failed. I have permission to push to the forked repository, so I cloned that repository, updated CHANGELOG, committed, and pushed. However, the workflow run again with this PR fails.

If there is a way to solve this problem, please let me know 🙏

@chad1008
Copy link
Contributor

Hey @t-hamano, thanks for the ping 😄

Looking at the job details, it looks like it's failing before it even gets around to actually checking the CHANGELOG... it's failing when the workflow tries to check out the repo for some reason 🤔

I'm not sure if there's an issue specific to this PR, or something that will impact any PR from a cloned repo, but I'll investigate.

In any event, the CHANGELOG entry you've added looks good, once the PR is approved you should be able to merge, ignoring the check. It's not a required check so it shouldn't block you, but drop me a ping if it gives you any trouble.

@t-hamano t-hamano self-requested a review April 14, 2023 03:35
@t-hamano
Copy link
Contributor

@chad1008

Thanks for your reply.

I tried submitting a PR from the forked repository in #49816, but the workflow still seems to fail. I have included the details in #49817.

Regardless of the cause, I would like to merge this PR as I believe the CHANGELOG has been updated correctly as well.

@t-hamano t-hamano changed the title fix: insufficient color contrast on hover in snackbar Snackbar: Fix insufficient color contrast on hover Apr 14, 2023
@t-hamano t-hamano merged commit 64ac377 into WordPress:trunk Apr 14, 2023
@github-actions
Copy link

Congratulations on your first merged pull request, @MahendraBishnoi29! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!

@desrosj
Copy link
Contributor

desrosj commented Aug 1, 2023

Hey @MahendraBishnoi29! If you have a moment, could you connect your GitHub account to your wordpress.org one? This change is slated to be included in the upcoming WordPress 6.3 release on August 8th, and it would be great if we could give you attribution by mentioning you on the credits screen. You can find more details on how contributions are tracked and how connecting GitHub accounts helps in this post on the Make WordPress Core blog.

@MahendraBishnoi29
Copy link
Contributor Author

Hey @MahendraBishnoi29! If you have a moment, could you connect your GitHub account to your wordpress.org one? This change is slated to be included in the upcoming WordPress 6.3 release on August 8th, and it would be great if we could give you attribution by mentioning you on the credits screen. You can find more details on how contributions are tracked and how connecting GitHub accounts helps in this post on the Make WordPress Core blog.

Done @desrosj

@sabernhardt
Copy link
Contributor

@MahendraBishnoi29 Could you share a link to your WordPress.org profile? I could not find it with my search methods.

@MahendraBishnoi29
Copy link
Contributor Author

here @sabernhardt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Components /packages/components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants