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 configuration option for visually hidden space on SummaryListComponent::ActionComponent #403

Closed
wants to merge 1 commit into from

Conversation

paulodeon
Copy link

As per issue #402. We are adding a configuration option to allow moving the space in the summary list actions from the visually shown to visually hidden text.

I'm not sure whether we should be using nbsp or a regular space here but I've put in nbsp for now as that was the recommendation

@netlify
Copy link

netlify bot commented Mar 1, 2023

Deploy Preview for govuk-components ready!

Name Link
🔨 Latest commit 42fd821
🔍 Latest deploy log https://app.netlify.com/sites/govuk-components/deploys/63ff5f2e45a04700087f9ce7
😎 Deploy Preview https://deploy-preview-403--govuk-components.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@frankieroberto
Copy link
Collaborator

I'm not sure this needs to be a configurable option... If it's needed we should always use it.

That said, the upstream issue suggests that it might be an issue even with the non-breaking space.

In the short term, could we do the small step of moving the existing space inside the <span class="govuk-visually-hidden"> rather than outside it? (This would bring the Rails component in alignment with the nunjucks version.)

@peteryates
Copy link
Member

peteryates commented Mar 1, 2023

PR looks good, as @frankieroberto says let's hold fire a while and see what happens upstream.

Many thanks for submitting @paulodeon

peteryates added a commit that referenced this pull request Mar 10, 2023
The final decision on how to properly handle the space between the
visible and hidden text hasn't been made yet but the general consensus
upstream is that it's better inside the visually hidden span than before
it.

This might be revisited again when a decision has been made and the
solution presented in #403 can be used instead.
peteryates added a commit that referenced this pull request Mar 10, 2023
The final decision on how to properly handle the space between the
visible and hidden text hasn't been made yet but the general consensus
upstream is that it's better inside the visually hidden span than before
it.

This might be revisited again when a decision has been made and the
solution presented in #403 can be used instead.
@peteryates
Copy link
Member

Closing this now as it seems shifting the space has done the trick.

@peteryates peteryates closed this Jul 13, 2023
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.

3 participants