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

Notice: Update dismiss button to match other Gutenberg UI #16926

Merged
merged 3 commits into from
Aug 7, 2019

Conversation

kjellr
Copy link
Contributor

@kjellr kjellr commented Aug 6, 2019

Currently the dismiss button in the Notice component is a little non-standard:

  • It uses the no dashicon, instead of no-alt (which is used in our modal and sidebar panel close buttons)
  • It is $dark-gray-300 by default (instead of the usual $dark-gray-500), and is red on hover, regardless of the type of notification.

This PR updates those to be more consistent.

Screenshots

Before:

Notice dismiss icon alongside the panel close icon:
gutenberg test_wp-admin_post-new php (2)

Hovers:
close-old

After:

Notice dismiss icon alongside the panel close icon:
gutenberg test_wp-admin_post-new php (1)

Hovers:
close-new

Testing

Pasting this into your console should produce all 4 standard dismissible notices.

wp.data.dispatch('core/notices').createNotice(
	'',
	'Default',
	{
		isDismissible: true,
	}
);
wp.data.dispatch('core/notices').createNotice(
	'warning',
	'Warning',
	{
		isDismissible: true,
	}
);
wp.data.dispatch('core/notices').createNotice(
	'success',
	'Success',
	{
		isDismissible: true,
	}
);
wp.data.dispatch('core/notices').createNotice(
	'error',
	'Error',
	{
		isDismissible: true,
	}
);

@kjellr kjellr added Needs Design Feedback Needs general design feedback. [Package] Notices /packages/notices labels Aug 6, 2019
@kjellr kjellr requested a review from mapk August 6, 2019 14:54
@kjellr kjellr self-assigned this Aug 6, 2019
Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@karmatosed karmatosed self-requested a review August 7, 2019 08:59
Copy link
Member

@karmatosed karmatosed left a comment

Choose a reason for hiding this comment

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

Approving design as this is a great unifying PR. Thanks for catching this.

@karmatosed karmatosed merged commit 612d8bc into master Aug 7, 2019
@github-actions github-actions bot added this to the Gutenberg 6.3 milestone Aug 7, 2019
@kjellr kjellr deleted the update/notice-close-button branch August 7, 2019 12:32
@ellatrix
Copy link
Member

ellatrix commented Aug 7, 2019

I'm seeing some js unit test error after this. Looks like you forgot to update a snapshot?

@ellatrix
Copy link
Member

ellatrix commented Aug 7, 2019

Travis CI - Pull Request Build Errored

This PR was merged while there were failing tests. :/

ellatrix added a commit that referenced this pull request Aug 7, 2019
jorgefilipecosta pushed a commit that referenced this pull request Aug 7, 2019
@jorgefilipecosta
Copy link
Member

Thank you for fixing this problem @ellatrix. During the code review, we had another problem making CI tests fail and I did not notice the snapshot, I'm sorry for the potential troubles caused.

@mapk
Copy link
Contributor

mapk commented Aug 7, 2019

Thanks for taking care of this Kjell. 🎉

gziolo pushed a commit that referenced this pull request Aug 29, 2019
* Update hover/active/focus color to dark gray.

* Update close button color to be darker.

This matches our other close buttons throughout the interface.

* Use no-alt instead of no dashicon.
gziolo pushed a commit that referenced this pull request Aug 29, 2019
gziolo pushed a commit that referenced this pull request Aug 29, 2019
* Update hover/active/focus color to dark gray.

* Update close button color to be darker.

This matches our other close buttons throughout the interface.

* Use no-alt instead of no dashicon.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Feedback Needs general design feedback. [Package] Notices /packages/notices
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants