javascript
javascript copied to clipboard
Add a dismissable Alert component with 4 different styles: info, warning, error & success.
Summary
Adds an Alert component to Yoast components. It has 4 different styles that can be set in its props: info, warning, error & success. It has an option to be dismissable, which will be saved in a cookie. The alert is based on this design: https://github.com/Yoast/design/issues/371#issuecomment-487953202
- Package(s) involved:
- Should the change be included in the package changelog?
- [ ] No
- [x] Yes
- Should the change be included in one or more plugin changelogs?
- [x] No
- [ ] Free
- [ ] Premium
- [ ] Other (please specify)
- Package changelog item (if applicable):
- Adds a dismissable Alert component with 4 different styles: info, warning, error & success.
Relevant technical choices:
- Added the component to the example App under the section 'components'.
- Removed the svg files and replaced them with inline generated svg files in the alert component.
- Wrapped the App in <IntlProvider locale="en"> to make the component work, as it uses react-intl.
- Added grunt to @yoast/style-guide to be able to build the colors.json file.
- Added all the required colors to
_colors.scss
. It's a lot of new colors so it makes the file a bit messy. Would like a second opinion on how to make this cleaner.
Test instructions
This PR can be tested by following these steps:
- Checkout this branch. Go into the apps/components folder and run yarn install, then yarn start
- Open the website on the provided port and go into the components tab.
- Here you should see the following 4 alerts:
- Inspect them and make sure they match the specifications here: https://github.com/Yoast/design/issues/371#issuecomment-487953202
- Close them and make sure they don't come back unless you delete the cookies :)
- If you want to be thorough you can also yarn link to myYoast and test the alerts there.
Impact check
UI changes
- [ ] This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.
Quality assurance
- [x] I have tested this code to the best of my abilities
- [ ] I have added unittests to verify the code works as intended
Fixes #331
CR 👍 Acceptance 🚧
1
Important: I think the method used for translations should be changed in favor of @wordpress/i18n
. Please see comment in the CR.
2
When used in Yoast SEO, the "close" button won't have any focus style when using Firefox. This is because the button inherits CSS rules from WordPress that remove the proprietary styles for ::-moz-focus-inner
. We should provide a custom focus style and not rely on the browsers native focus style. See https://github.com/Yoast/bugreports/issues/511
3 There are no tests. Not sure if this was agreed internally. Maybe add a basic snapshot test?
4 The "close" button size is 20 by 20 pixels. I'd say that on mobile it's too small. We should consider to make it bigger on small screens. This would require design feedback and I guess it can be split in a separate issue.
5
Note: importing and using the Alert on Yoast SEO was a painful experience for me. Maybe I'm missing some important detail but between linking the monorepo, yarn install & Co. I was getting an error Module not found: Error: Can't resolve 'js-cookie' in '/Users/andrea/wp/plugins/wordpress-seo/node_modules/@yoast/components/src'
. No idea why, as it works in the standalone example. I had to manually yarn add js-cookie
to Yoast SEO. I'd suggest to double check this point.
The Alert tested on a Yoast SEO page:

@afercia Thanks for your extensive review. I have implemented the changes as following:
-
You're right. I had to gain some context on the translation dependencies. I have replaced react-intl with @wordpress/i18n.
-
I let this how it is and assume the bugreport you filed will fix it?
-
I added four snapshot tests.
-
I think this should be a separate issue as it would indeed involve a design decision.
-
I checked with Herre and this should not be an issue. The 'js-cookie' package is added to @yoast/components in this PR. Are all the files build correctly?
Thanks @Dieterrr !
4 Will create a new issue
5 Maybe I'm missing something. Just tested again. What I do is:
- switch the javascript repo to this branch and run
yarn
on the project root - on wordpress-seo trunk, run
yarn link-monorepo
- run
yarn && grunt build:css && grunt build:js && grunt webpack:buildProd
- error:
ERROR in ./node_modules/@yoast/components/src/Alert.js
Module not found: Error: Can't resolve 'js-cookie' in '/Users/andrea/wp/plugins/wordpress-seo/node_modules/@yoast/components/src'
@ ./node_modules/@yoast/components/src/Alert.js 22:16-36
@ ./node_modules/@yoast/components/src/index.js
@ ./js/src/components.js
The only way to fix it for me is by adding js-cookie
manually: yarn add js-cookie
. I also tried deleting all the node_modules
directories in all projects, no luck.
Even after I manually add js-cookie
and try to import and use the Alert in the Yoast SEO plugin (see attached diff), I don't get the colors. this.options.color
and this.options.background
. The colors.json file doesn't contain these new colors. Is this an issue related to versions>

I changed the typo for the link color.
Acceptance 🚧 My local environment fails when I try to test in MyYoast. Could you look at this and make sure it's not because of the PR?
I merged with develop and fixed the snapshots.
I'm currently getting the following error in myYoast when I link it.
styled-components.browser.cjs.js:2479 It looks like there are several instances of 'styled-components' initialized in this application. This may cause dynamic styles not rendering properly, errors happening during rehydration process and makes your application bigger without a good reason.
See https://s-c.sh/2BAXzed for more info.
(anonymous) @ styled-components.browser.cjs.js:2479
(anonymous) @ styled-components.browser.cjs.js:2503
__webpack_require__ @ bootstrap 672f71022272ce7e07ff:582
fn @ bootstrap 672f71022272ce7e07ff:106
(anonymous) @ Button.js:2
__webpack_require__ @ bootstrap 672f71022272ce7e07ff:582
fn @ bootstrap 672f71022272ce7e07ff:106
(anonymous) @ Loader.js:12
__webpack_require__ @ bootstrap 672f71022272ce7e07ff:582
fn @ bootstrap 672f71022272ce7e07ff:106
(anonymous) @ Loader.js:2
__webpack_require__ @ bootstrap 672f71022272ce7e07ff:582
fn @ bootstrap 672f71022272ce7e07ff:106
(anonymous) @ SitesPage.js:11
__webpack_require__ @ bootstrap 672f71022272ce7e07ff:582
fn @ bootstrap 672f71022272ce7e07ff:106
(anonymous) @ SitesPage.js:5
__webpack_require__ @ bootstrap 672f71022272ce7e07ff:582
fn @ bootstrap 672f71022272ce7e07ff:106
(anonymous) @ Menu.js:11
__webpack_require__ @ bootstrap 672f71022272ce7e07ff:582
fn @ bootstrap 672f71022272ce7e07ff:106
(anonymous) @ App.js:20
__webpack_require__ @ bootstrap 672f71022272ce7e07ff:582
fn @ bootstrap 672f71022272ce7e07ff:106
(anonymous) @ index.js:3
(anonymous) @ index.js:87
__webpack_require__ @ bootstrap 672f71022272ce7e07ff:582
fn @ bootstrap 672f71022272ce7e07ff:106
(anonymous) @ bootstrap 672f71022272ce7e07ff:628
__webpack_require__ @ bootstrap 672f71022272ce7e07ff:582
(anonymous) @ bootstrap 672f71022272ce7e07ff:628
(anonymous) @ bootstrap 672f71022272ce7e07ff:628
Show 3 more frames
styled-components.browser.cjs.js:213 Uncaught Error: Trying to insert a new style tag, but the given Node is unmounted!
- Are you using a custom target that isn't mounted?
- Does your document not have a valid head element?
- Have you accidentally removed a style tag manually?
@afercia I have reproduced the issue with the colors. When I log it instead of a color string it is undefined. I wonder if it's related to linking it because if I open the json file I the colors are there. I will look into it and into the js-cookie problem.
I have put this in blocked and created an issue as the issues with webpack and linking the javascript monorepo were too time consuming and outside of the scope of this PR.
Here is the issue: https://github.com/Yoast/my-yoast/issues/2840
I don't think the current linking between MyYoast and the javascript monorepo is in a usable state and would propose not using it until these issues are fixed.
Noting there's now a new PR https://github.com/Yoast/javascript/pull/349 and maybe this one should be closed.