Implement SeoAnalysis and ReadabilityAnalysis components #492

igorschoester · 2018-05-03T14:56:24Z

Summary

This PR can be summarized in the following changelog entry:

Originally planned to create two components: SEO Analysis and Readability Analysis. However, this change has been reverted and now this PR creates support for using the smaller components which later be used to build the functionality of the original two planned components.

Relevant technical choices:

This PR became a little more involved than the issue would indicate.

Added the @wordpress/i18n package to replace react-intl.
Extracted a LanguageNotice component from ContentAnalysis.
Removed the HelpText (and the LanguageNotice) from ContentAnalysis.
Updated the HelpText & KeywordInput components.
Replaced the react-intl with the @wordpress/i18n variant in the makeOutboundLink util.
Updated KeywordExample to work correctly with new changes.
Added onChange to the KeywordInput props.
Added 'Focus keyword:' label to KeywordInput.
Tweaked design of components to better match proposed design:
-- Changed the size of the collapsible header arrows.
-- Made the header the "correct" blue: blue_medium, aka #1e8cbe.
-- The traffic light icons are resized to be slightly smaller
-- The blue headings should be slightly smaller: Same size as the text in the p element (13px) and the 'Focus keyword:' heading.

Test instructions

This PR can be tested by following these steps:

Normal pull: git checkout stories/475-implement-seo-analysis-component.
Normal install: yarn install.
Run the local dev server of yoast-components: yarn start.
In the tab 'Content Analysis' the language notice should be at the top and the style changes should be present in the content analysis below.
In the tab 'Keyword' the HelpText should be visible above and the keyword input below. The keyword input should throw an error when a comma is entered. Also, there should be a 'Focus keyword' label.
Checkout its sibling PR in WordPress-SEO: Moved language notice props to seperate component wordpress-seo#10438.
Normal install: yarn install, yarn link yoast-components, grunt build
Run the local dev server in WordPress-SEO: yarn start.
Visit the edit-post page.
Check the readability analysis in the metabox.
Check if the language notice shows as it should.

Fixes #475

IreneStr · 2018-05-04T11:10:34Z

@igorschoester

The style of KeywordInput might not be enough. Should this be an auto height input?

The inputfield should grow vertically when the input is too long for one line

The up and down arrows in front on the analysis points look fatter. Should this be updated too?

Yes. And it's no problem that that means that it will change in the metabox as well.

Added an already obsolete Separator component. Should I remove this?

Yes

Are the links for the HelpText the same in both SEO & readability?

Please ask content or support what are the best posts to use for the links. Please ask Annelieke to create shortlinks for any newly introduced links.

boblinthorst · 2018-07-11T07:47:35Z

A concern I have is what'll happen to the current metabox upon merging. Many style changes will be made to make everything look good on the sidebar, but in the process, the original metabox will have the same changes. And what looks good in a sidebar, doesn't necessarily look good in the metabox.

I've discussed this with Igor, and concluded that, in the event we make the original metabox unworkable, we should just add the old code and keep that part intact, and make sure this new sidebar is it's own contained code.

…d the color assignment of the header

afercia · 2018-07-19T10:45:59Z

app/KeywordExample.js

 	 */
 	render() {
 		return (
 			<Container>
+				<HelpText text={ [
+					"Enter the search term you'd like this post to be found with and see how it would rank. ",
+					<HelpTextLink key="1" href="https://yoa.st/" target="_blank" rel="noopener">


target="_blank" rel="noopener" should be removed as they're overriding the makeOutboundLink() defaults. Notice noreferrer is not rendered. Remove these two attributes and makeOutboundLink() will work as expected.

My mistake, missed that. Apologies.

afercia · 2018-07-19T11:28:41Z

composites/Plugin/Shared/components/KeywordInput.js

+`;
+
+const KeywordField = styled.input`
+	border: 3px solid initial;


Why initial? Doesn't seem to work, at least in the shorthand syntax. Chrome and Firefox discard the whole border property (please check in your browser's inspector). The single property border-color: initial; seems to work at least in Chrome and Firefox but I guess it's not what we want, because it looks different across browsers.
For other input fields borders we're using $color_input_border or $color_grey or directly #ddd to be consistent with WordPress. Note: the contrast ratio is not sufficient but this should be addressed separately.

afercia · 2018-07-19T11:35:14Z

composites/Plugin/Shared/components/HelpText.js

+	margin-top: 0;
+
+	a {
+		color: ${ colors.$color_snippet_focus };


This point still needs an answer. @hedgefield seems to me we should let links inherit the default WordPress colors for links (default: #0073aa), as we're already doing for all the links in the meta box. Why we should use a different color?

Ahh that's the default link color, I thought it was the Yoast Blue (the hex code is very similar), which isn't in our colors scss, so I changed it to that. But yes if that's the link color we use everywhere, we should use that.

afercia · 2018-07-19T11:35:40Z

composites/Plugin/Shared/components/LanguageNotice.js

+`;
+
+const ChangeLanguageLink = makeOutboundLink( styled.a`
+	color: ${ colors.$color_snippet_focus };


Same consideration for links colors, see above

afercia · 2018-07-19T11:36:58Z

CR 🚧

A few considerations to address.

maartenleenders · 2018-07-23T11:04:41Z

app/ContentAnalysisWrapper.js

-				<a key="1" href="https://yoa.st/" target="_blank" rel="noopener">Learn more about the Content Analysis Tool.</a>,
-			] }
-		/>
+		<div>


~~Could be a <React.Fragment>, div is not necessary~~

never mind this is the example app, doesn't matter that much then

maartenleenders · 2018-07-23T12:40:33Z

CR 👍

maartenleenders · 2018-07-25T08:12:06Z

Acceptance 🚧

Since the ContentAnalysis component was touched (LanguageNotice was split out from it), wordpress-seo needs to be updated to render the new independend LanguageNotice.
The styling in the ContentAnalysis seems to be slightly below our standards, but that is my opinion. I think we'd do well to give everything a little more 'breathing room' with padding/margins. @hedgefield has offered to sit in on a little styling session to improve things. I would also be fine if you split this up into a new issue, as this PR has become quite big.

boblinthorst · 2018-07-25T10:12:33Z

I've created a sibling pr to deal with the languageNotice. I've decided to create a seperate issue for the styling issues. Mostly because this pr is way too big, but also partially because the styling of the content analysis hasn't been touched that much in this pr, so addressing this here would add even more responsibilities to this pr.

The seperate issue: #671

jcomack

CR done 🏗

jcomack · 2018-07-25T10:57:19Z

app/ContentAnalysisWrapper.js

@@ -1,6 +1,7 @@
 import React from "react";

 import ContentAnalysis from "../composites/Plugin/ContentAnalysis/components/ContentAnalysis";
+import LanguageNoticeWrapper from "./LanguageNoticeWrapper.js";


I believe @atimmer mentioned something about not importing files in this manner. Not sure if that's relevant to this or not.

Anton did say something about importing in this style. But i believe he referred to to importing yoast-components from wordpress-seo.

When importing components from yoast-components in wpseo, exporting the component in the index.js file of yoast-components, allows you to import it in wordpress-seo directly from yoast-components.

I didn't do that properly in this pull, so the remark was still valid.

I'll still check with Anton if a similar way of importing is possible within yoast-components itself.

Update: Anton confirmed the new way of importing only applies to when using yoast-components outside the project.

jcomack · 2018-07-25T10:58:33Z

app/KeywordExample.js

@@ -5,32 +5,92 @@ import styled from "styled-components";
 // Internal dependencies.
 import KeywordInput from "../composites/Plugin/Shared/components/KeywordInput";
 import SynonymsSection from "../composites/Plugin/Synonyms/components/SynonymsSection";
+import HelpText from "../composites/Plugin/Shared/components/HelpText.js";


See previous comment regarding this style of important components.

jcomack · 2018-07-25T11:07:06Z

app/KeywordExample.js

 	 */
 	render() {
 		return (
 			<Container>
+				<HelpText text={ [
+					"Enter the search term you'd like this post to be found with and see how it would rank. ",
+					<HelpTextLink key="1" href="https://yoa.st/">


Is this link purely an example?

This example can only be reached in the standalone. Which you can visit by running the dev-server in yoast-components (yarn start) and going to localhost:3333 in your browser.

jcomack · 2018-07-25T11:07:32Z

app/LanguageNoticeWrapper.js

+import React from "react";
+import styled from "styled-components";
+
+import LanguageNotice, { languageNoticePropType } from "../composites/Plugin/Shared/components/LanguageNotice.js";


See previous comment regarding the importing of files.

jcomack · 2018-07-25T11:29:09Z

composites/Plugin/Shared/components/KeywordInput.js

-	 * @param {String}    	props.id       The id of the KeywordInput.
-	 * @param {IconsButton} props.label    The label of the KeywordInput.
-	 * @param {boolean}     props.keyword  The initial keyword passed to the state.
+	 * @param {Object}   props           The props for this input field component.


I'd vouch for being consistent with the usage of names... So instead of speaking of 'this input field component' I'd say 'The props for the KeywordInput'.

jcomack · 2018-07-25T11:47:12Z

app/LanguageNoticeWrapper.js

+ * @param {Object} props                    The props for this language notice wrapper.
+ * @param {string} props.changeLanguageLink The URL where the language can be changed.
+ * @param {bool}   props.canChangeLanguage  Whether or not the language can be changed.
+ * @param {string} props.language           The current set language.


current should be currently.

jcomack · 2018-07-25T11:47:42Z

composites/Plugin/Shared/components/LanguageNotice.js

+ * @param {Object} props                    The props for this language notice wrapper.
+ * @param {string} props.changeLanguageLink The URL where the language can be changed.
+ * @param {bool}   props.canChangeLanguage  Whether or not the language can be changed.
+ * @param {string} props.language           The current set language.


current should be currently.

jcomack · 2018-07-25T11:51:48Z

composites/Plugin/Shared/components/LanguageNotice.js

+export default class LanguageNotice extends PureComponent {
+	/**
+	 * Renders the language notice. Provides a link to a setting page in case of
+	 * administrator, a notice to contact an administrator otherwise.


I'd recommend rewriting this to something like the following:

'Renders the language notice. Provides a link to a setting page if the currently set language can be changed. Provides a message to contact an administrator otherwise.'

jcomack · 2018-07-25T11:52:27Z

composites/Plugin/Shared/components/LanguageNotice.js

+
+		// Determine the correct text.
+		let text = canChangeLanguage ? canChangeLanguageText : canNotChangeLanguageText;
+		// Replace the %s with a strong marked language.


Missing whitespace above comment.

jcomack · 2018-07-25T11:52:30Z

composites/Plugin/Shared/components/LanguageNotice.js

+		let text = canChangeLanguage ? canChangeLanguageText : canNotChangeLanguageText;
+		// Replace the %s with a strong marked language.
+		text = sprintf( text, `{{strong}}${ language }{{/strong}}` );
+		// Replace the strong marking with an actual ReactComponent.


Missing whitespace above comment.

maartenleenders · 2018-07-26T09:24:45Z

CR 👍

maartenleenders · 2018-07-26T09:50:58Z

Acceptance 👍

igorschoester added 12 commits May 2, 2018 15:11

Add simple Separator component

1d73645

Start with SeoAnalysis

d97e02f

Update HelpText component

82dd1b0

Add @wordpress/i18n package

ae79658

Remove react-intl makeOutboundLink

5f910a5

Extract LanguageNotice component

2f5fe3c

Update HelpText

25a26dc

Refactor KeywordInput prop type

e8e8f06

Remove LanguageNotice and HelpText from ContentAnalysis

49ef268

Add ReadabilityAnalysis and adapt SeoAnalysis

1ae3bb5

Make label optional, refactor handleChange and style in KeywordInput

7cd6ac0

Add small docs and style changes

aeb6d76

Dieterrr changed the base branch from feature/content-analysis-in-sidebar to develop July 9, 2018 09:58

Dieterrr changed the base branch from develop to feature/content-analysis-in-sidebar July 9, 2018 09:59

This was referenced Jul 9, 2018

Implement React focus keyword field in metabox Yoast/wordpress-seo#10297

Closed

Create an SEO analysis container Yoast/wordpress-seo#10303

Closed

Create and implement a readability analysis container Yoast/wordpress-seo#10304

Closed

Dieterrr added 2 commits July 10, 2018 12:28

Merge branch 'develop' into stories/475-implement-seo-analysis-component

68dc65f

Fixed language notice text

05552a8

Dieterrr changed the base branch from feature/content-analysis-in-sidebar to develop July 10, 2018 11:59

Dieterrr added 4 commits July 10, 2018 14:20

Merge branch 'develop' into stories/475-implement-seo-analysis-component

5577510

Deleted the separator component

0e7d3de

Changed link color to correct color blue

b65bb4b

Added 'Focus keyword:' and 'Analysis results' headings

1f4ed34

boblinthorst added 4 commits July 11, 2018 10:37

Changed shade of blue

f6c19dc

Adjusted the size of the header and its accompanying icons. Also move…

d778f00

…d the color assignment of the header

Made use of focus keyword's built in label, instead of adding a new one

4c6fc9d

Added onChange property to keywordInput

3b573d2

boblinthorst added 5 commits July 17, 2018 11:47

Adressed cr by @afercia

5cf58a9

Merge branch 'develop' into stories/475-implement-seo-analysis-component

5663b6b

Changed docblock I forgot

b70f1a9

switched spaces for a tab

b7fb029

Added periods to docblocks

97e489e

afercia reviewed Jul 19, 2018

View reviewed changes

boblinthorst added 4 commits July 20, 2018 08:49

Adressed two mistakes made during cr adressing

c765853

Removed color styling for links

1c63ddd

Added snapshots

b3bf176

removed unused import

e39ae85

maartenleenders reviewed Jul 23, 2018

View reviewed changes

Fixed synonymsField in the standalone.

bae7c89

boblinthorst mentioned this pull request Jul 25, 2018

Styling improvements for the Analysis results. #671

Closed

jcomack suggested changes Jul 25, 2018

View reviewed changes

boblinthorst added 4 commits July 25, 2018 17:46

Addressed non-import comments from review

88ab6fa

Adressed spelling mistake in comment

26941e3

Merge branch 'develop' into stories/475-implement-seo-analysis-component

3bfa1b2

Added languageNotice as export

98fb1ad

Prepare additional export

60031f0

maartenleenders merged commit a7059f4 into develop Jul 26, 2018

maartenleenders deleted the stories/475-implement-seo-analysis-component branch July 26, 2018 09:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement SeoAnalysis and ReadabilityAnalysis components #492

Implement SeoAnalysis and ReadabilityAnalysis components #492

igorschoester commented May 3, 2018 •

edited by boblinthorst

Loading

IreneStr commented May 4, 2018

boblinthorst commented Jul 11, 2018

afercia Jul 19, 2018

boblinthorst Jul 20, 2018

afercia Jul 19, 2018

afercia Jul 19, 2018

hedgefield Jul 23, 2018

afercia Jul 19, 2018

afercia commented Jul 19, 2018

maartenleenders Jul 23, 2018 •

edited

Loading

maartenleenders commented Jul 23, 2018

maartenleenders commented Jul 25, 2018 •

edited by boblinthorst

Loading

boblinthorst commented Jul 25, 2018 •

edited

Loading

jcomack left a comment

jcomack Jul 25, 2018

boblinthorst Jul 26, 2018 •

edited

Loading

jcomack Jul 25, 2018

jcomack Jul 25, 2018

boblinthorst Jul 25, 2018 •

edited

Loading

jcomack Jul 25, 2018

jcomack Jul 25, 2018

jcomack Jul 25, 2018

jcomack Jul 25, 2018

jcomack Jul 25, 2018

jcomack Jul 25, 2018

jcomack Jul 25, 2018

maartenleenders commented Jul 26, 2018

maartenleenders commented Jul 26, 2018

Implement SeoAnalysis and ReadabilityAnalysis components #492

Implement SeoAnalysis and ReadabilityAnalysis components #492

Conversation

igorschoester commented May 3, 2018 • edited by boblinthorst Loading

Summary

Relevant technical choices:

Related links

Test instructions

IreneStr commented May 4, 2018

boblinthorst commented Jul 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afercia commented Jul 19, 2018

maartenleenders Jul 23, 2018 • edited Loading

Choose a reason for hiding this comment

maartenleenders commented Jul 23, 2018

maartenleenders commented Jul 25, 2018 • edited by boblinthorst Loading

boblinthorst commented Jul 25, 2018 • edited Loading

jcomack left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boblinthorst Jul 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boblinthorst Jul 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maartenleenders commented Jul 26, 2018

maartenleenders commented Jul 26, 2018

igorschoester commented May 3, 2018 •

edited by boblinthorst

Loading

maartenleenders Jul 23, 2018 •

edited

Loading

maartenleenders commented Jul 25, 2018 •

edited by boblinthorst

Loading

boblinthorst commented Jul 25, 2018 •

edited

Loading

boblinthorst Jul 26, 2018 •

edited

Loading

boblinthorst Jul 25, 2018 •

edited

Loading