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.

Sign up for GitHub

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

Jump to bottom

Pluralise radio component #388

Merged
merged 2 commits into from
Dec 22, 2017
Merged

Pluralise radio component #388

merged 2 commits into from
Dec 22, 2017

Conversation

kr8n3r
Copy link

@kr8n3r kr8n3r commented Dec 14, 2017

We've agreed to make component names plural where there they are always
or nearly always used in groups (radios, breadcrumbs and checkboxes).

This PR updates:

Readme wording
Class names
Macro
Folder name
YAML file name
Macro name where used
Updates the reference in the all package json and scss files

Part of Trello ticket

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-388 December 14, 2017 10:34 Inactive
@kr8n3r
Copy link
Author

kr8n3r commented Dec 14, 2017

@36degrees i assume same thing with wrapping div class names?

@36degrees
Copy link
Contributor

Yeah, I think so.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-388 December 15, 2017 09:16 Inactive
alex-ju
alex-ju approved these changes Dec 15, 2017
View reviewed changes
Copy link
Contributor

@alex-ju alex-ju left a comment

Choose a reason for hiding this comment

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

Looking good.
The only issue is the markup indentation, on which I know @igloosi is looking at right now.

@kr8n3r kr8n3r force-pushed the pluralise-radio-component branch from 2aaf30d to a6e8b0c Compare December 15, 2017 15:43
@kr8n3r kr8n3r mentioned this pull request Dec 15, 2017
Merged
@alex-ju
Copy link
Contributor

alex-ju commented Dec 20, 2017 •
edited
Loading

This looks fine. It just needs rebasing.

Pluralise radio component
448b501 
We've agreed to make component names plural where there they are always
or nearly always used in groups (radios, breadcrumbs and checkboxes).
@kr8n3r kr8n3r force-pushed the pluralise-radio-component branch from a6e8b0c to 4a0920e Compare December 22, 2017 14:12
Update HTML structure and readme
5823e0e 
Wrap all radio items in  .govuk-c-radios class
@kr8n3r kr8n3r force-pushed the pluralise-radio-component branch from 4a0920e to 5823e0e Compare December 22, 2017 14:13
@kr8n3r
Copy link
Author

kr8n3r commented Dec 22, 2017

before fieldset changes
screen shot 2017-12-22 at 14 04 40

after fieldset changes
screen shot 2017-12-22 at 14 12 17

@kr8n3r kr8n3r merged commit 366a1fe into master Dec 22, 2017
@kr8n3r kr8n3r deleted the pluralise-radio-component branch December 22, 2017 14:17
This was referenced Jan 10, 2018
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alex-ju alex-ju alex-ju approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kr8n3r @36degrees @alex-ju @govuk-design-system-ci