Skip to content
This repository was archived by the owner on Oct 4, 2022. It is now read-only.

Add an Alert component #349

Merged
merged 14 commits into from
Sep 10, 2019
Merged

Conversation

igorschoester
Copy link
Member

@igorschoester igorschoester commented Sep 2, 2019

Summary

This PR can be summarized in the following changelog entry:

  • [@yoast/components] Adds an Alert component.
  • [@yoast/style-guide] Adds new status colors: error, info, success, warning.

Relevant technical choices:

  • Based on the MyYoast implementation. Should be similar in usage. The cookie and dismiss should now be controlled from outside this component.
  • Adds the colors to the style-guide in a hopefully reusable fashion.

Test instructions

This PR can be tested by following these steps:

  • Run the components app.
  • Check the different type of alerts in the components 'tab'.
  • Check the dismissable alert.
  • The should look like the design.
  • Check in Right To Left: spacing of the icons on the correct sides.

Impact check

  • This PR affects the following parts of the plugin, which may require extra testing:
    *

Screenshot

alerts

UI changes

  • This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Quality assurance

  • I have tested this code to the best of my abilities
  • I have added unittests to verify the code works as intended

Partially fixes: Yoast/wordpress-seo#13173

@igorschoester igorschoester added the changelog: enhancement Needs to be included in the 'Enhancements' category in the changelog label Sep 2, 2019
* Use `×` as dismiss icon.
* Overwrite the content's anchor color.
* Moved the colors to the style guide repo.
@hwinne
Copy link
Contributor

hwinne commented Sep 5, 2019

CR Done 👍

@hwinne hwinne removed their assignment Sep 5, 2019
@johannadevos johannadevos self-assigned this Sep 5, 2019
@abotteram abotteram assigned afercia and unassigned johannadevos Sep 9, 2019
@afercia
Copy link
Contributor

afercia commented Sep 9, 2019

About the translatable string in the default prop, see for example Yoast/wordpress-seo#11705 (comment)

Seems the default needs to be provided within the render.

@afercia
Copy link
Contributor

afercia commented Sep 9, 2019

I made several changes so I think it's better someone else does final acceptance. Will move back to the proper lane.

Screenshots:

Single line

Screenshot 2019-09-09 at 16 00 51

Multiple lines and focus style for the dismiss button.

Screenshot 2019-09-09 at 15 53 41

This is based on the provided design on https://github.com/Yoast/design/issues/392#issuecomment-515051908

  • 2px border #007FFF
  • 3px outer border #007FFF with an opacity of 0.25

Note: It is not possible to make the dismiss button alignment have a pixel-perfect alignment because:

  • alignment with the single line text matters
  • the × character changes depending on the font used and the font internal metrics. See for example in the plugin, where there's a different font-family thus the alignment is different.

Noted on the design issue, pending feedback.

In the plugin:

Screenshot 2019-09-09 at 16 14 56

Screenshot 2019-09-09 at 16 15 22

Screenshot 2019-09-09 at 16 17 32

In the last screenshot you can clearly see the vertical alignment is different. The only reason for this is the font face internal metrics. As noted in the design issue, the only option I can think of to avoid this is to use an SVG icon.

@afercia
Copy link
Contributor

afercia commented Sep 9, 2019

Re the × the decision is to use the current SVG icon, see feedback on https://github.com/Yoast/design/issues/371#issuecomment-529517547

@johannadevos
Copy link
Contributor

Acceptance: 👍

@johannadevos johannadevos added this to the 12.1 milestone Sep 10, 2019
@johannadevos johannadevos merged commit 922f79d into develop Sep 10, 2019
@johannadevos johannadevos deleted the 13173-add-notification-no-organization branch September 10, 2019 11:15
@manuelaugustin manuelaugustin modified the milestones: 12.1, 12.2 Sep 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
changelog: enhancement Needs to be included in the 'Enhancements' category in the changelog underestimated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a notification when users haven't set information for their Organization
6 participants