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

Global Notices: fixed notice__action for global notices #3122

Merged
merged 1 commit into from
Feb 5, 2016

Conversation

rickybanister
Copy link

The fade background was overflowing the round corners and the font-size was incorrect and the padding was off. I also reorganized the notices stylesheet to make more sense.

Closes #3119

cc @mtias @scruffian

Before:

After:
screen shot 2016-02-05 at 2 04 11 pm

The fade background was overflowing the round corners and the font-size was incorrect and the padding was off. I also reorganized the notices stylesheet to make more sense.
@rickybanister rickybanister added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR Components labels Feb 5, 2016
@rickybanister rickybanister self-assigned this Feb 5, 2016
@alternatekev
Copy link
Contributor

Visuals look good to me. Works as expected.

@alternatekev alternatekev removed the [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR label Feb 5, 2016
@alternatekev
Copy link
Contributor

There are some odd paddings in there:

  • 9px
  • 11px
  • 13px

Could these be made to be closer to our grid by changing them to

  • 8px
  • 12px
  • 12px

@alternatekev alternatekev added [Status] Needs Author Reply and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Feb 5, 2016
@alternatekev
Copy link
Contributor

Aside from the padding values, the code looks good to me too.

@rickybanister
Copy link
Author

The padding values get us to 40px overall height...shrug

rickybanister pushed a commit that referenced this pull request Feb 5, 2016
Global Notices: fixed notice__action for global notices
@rickybanister rickybanister merged commit e702bbe into master Feb 5, 2016
@rickybanister rickybanister deleted the fix/global-notice-action-shadow branch February 5, 2016 22:18
@mtias
Copy link
Member

mtias commented Feb 9, 2016

Looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants