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

Add guidance for divider and "none of these" to Checkboxes component #1535

Merged
merged 2 commits into from
Jun 24, 2021

Conversation

frankieroberto
Copy link
Contributor

@frankieroberto frankieroberto commented Feb 25, 2021

This updates the GOV.UK Design System website with guidance on the use of a "None of these" option for the Checkboxes component.

➡️ Preview of the Checkboxes component with new guidance

Uses a pre-release version of govuk-frontend, currently open as PR #2151.

NOTE: Do not merge until the govuk-frontend PR has been merged and released, and then update this PR to use the released version.

@govuk-design-system-ci
Copy link
Collaborator

govuk-design-system-ci commented Feb 25, 2021

✔️ You can preview this change here:
Built without sensitive environment variables

🔨 Explore the source changes: bb69762

🔍 Inspect the deploy log: https://app.netlify.com/sites/govuk-design-system-preview/deploys/60d461cba3803500075b003b

😎 Browse the preview: https://deploy-preview-1535--govuk-design-system-preview.netlify.app

Copy link
Contributor

@calvin-lau-sig7 calvin-lau-sig7 left a comment

Choose a reason for hiding this comment

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

Added suggestions to guidance content ahead of going to review by the Design System working group.

src/components/checkboxes/index.md.njk Outdated Show resolved Hide resolved
src/components/checkboxes/index.md.njk Outdated Show resolved Hide resolved
@frankieroberto
Copy link
Contributor Author

@calvin-lau-sig7 one other thing, I wasn't sure where best to place the error message guidance for the None option. One the one hand, it probably makes sense for it to be alongside the other error message guidance (which is where I put it), but on the other, it does make referring to it within the section a bit clumsy. I did have "(see below)" initially, but that felt worse... 🤔

Screenshot 2021-04-09 at 11 34 53

Copy link
Contributor

@calvin-lau-sig7 calvin-lau-sig7 left a comment

Choose a reason for hiding this comment

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

A couple of copy points that came up in the working group review (so far).

@frankieroberto frankieroberto changed the title WIP: Add divider and "none of these" to Checkboxes Add divider and "none of these" to Checkboxes May 27, 2021
@frankieroberto frankieroberto changed the title Add divider and "none of these" to Checkboxes DO NOT MERGE: Add divider and "none of these" to Checkboxes May 27, 2021
@frankieroberto frankieroberto marked this pull request as ready for review May 27, 2021 10:00
Copy link
Contributor

@EoinShaughnessy EoinShaughnessy left a comment

Choose a reason for hiding this comment

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

Content looks good! Added a few comments.

@36degrees 36degrees force-pushed the add-divider-and-none-of-thse branch from 022fe1a to 337429c Compare June 11, 2021 13:22
@36degrees 36degrees changed the title DO NOT MERGE: Add divider and "none of these" to Checkboxes Add guidance for divider and "none of these" to Checkboxes component Jun 18, 2021
Copy link
Contributor

@calvin-lau-sig7 calvin-lau-sig7 left a comment

Choose a reason for hiding this comment

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

Guidance text looks good to me! I've made one minor suggestion for better clarity.

src/components/checkboxes/index.md.njk Outdated Show resolved Hide resolved
@lfdebrux lfdebrux force-pushed the add-divider-and-none-of-thse branch from f56715f to bb69762 Compare June 24, 2021 10:43
@lfdebrux lfdebrux merged commit 2ceb9f0 into alphagov:main Jun 24, 2021
@frankieroberto frankieroberto deleted the add-divider-and-none-of-thse branch June 24, 2021 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

6 participants