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

CS-2160- Add new component: Topic Takeaway #612

Merged
merged 18 commits into from
Apr 3, 2018

Conversation

sirleech
Copy link
Contributor

@sirleech sirleech commented Mar 29, 2018

@sirleech sirleech requested review from pattyde, joolswood and alex-page and removed request for pattyde and joolswood March 29, 2018 21:38
@sirleech sirleech closed this Mar 29, 2018
@sirleech sirleech reopened this Mar 31, 2018
@govau govau deleted a comment from sirleech Apr 3, 2018
@govau govau deleted a comment from sirleech Apr 3, 2018
previouslink: /content-strategy/set-goals-measure-success/
nextheader: "Do next: Remove content"
nexttext: Clean up phase
nextlink: /content-strategy/remove-content/
Copy link
Contributor

Choose a reason for hiding this comment

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

Some of there are difficult to read maybe consider using _ to split words

Copy link
Contributor

Choose a reason for hiding this comment

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

I would split these by previous and next. First column and second column can be confusing for users entering in content.


.takeawayWhattodoLink {
padding-top: 20px;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We use BEM naming conventions. These will need to be fixed up as they are very inconsistent with the other class names conventions on the site. Make sure we don't use camel case it should be a combination of __ and - for BEM.

We also have a sass function for spacing uikit-space(). For now lets not use it as you will replace it when you update the uikit to the latest version however when writing out pixel values such as 14px and 20px that will cause problems in the future.

Whenever we use a colour value #666 make sure we use the correct variable in the $uikit-colour-Border.

There is also inconsistent white space which is killing me 😬 !!!! Lets make sure we use editorconfig and have consistent white space :)

*/
const TopicTakeaway = ( page ) => {
return (
<div className={ `uikit-body uikit-grid` } >
Copy link
Contributor

Choose a reason for hiding this comment

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

Because we don't use any variables this can be simplified to className="uikit-body uikit-grid"

@alex-page alex-page merged commit 96e772d into govau:test-staging Apr 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants