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

Fix alignment of conditional content beneath radio buttons and checkboxes #50

Conversation

peteryates
Copy link
Member

The conditional content, as explained in #48, was inserted in the wrong place causing it to render incorrectly. It has been moved so it's now in the correct position:

Screenshot 2019-10-09 at 19 59 54

Screenshot 2019-10-09 at 18 52 57

Nokogiri documents, by default, only allow for a single root element. As
we're going to be moving the conditional content to the same level as
the associated form control container we need to use
Nokogiri::HTML::DocumentFragment to parse our parsed_subject memoized
helper as it supports multi-root 'fragments' of HTML

Refs #48
The custom matcher is required because using the normal css selectors
it's impossible to ensure the element we find is at the root of the
fragment.

Refs #48
This bug was caused by the conditional content being inserted at the
wrong level, it was a child of the `govuk-checkboxes__item` but should
have been a sibling.

Fixes #48
Copy link
Contributor

@tvararu tvararu left a comment

Choose a reason for hiding this comment

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

Nice! ✨

@peteryates peteryates merged commit 6002e49 into master Oct 10, 2019
@peteryates peteryates deleted the move-conditional-content-to-same-level-as-controlling-element branch October 10, 2019 07:37
peteryates added a commit that referenced this pull request Jan 9, 2020
…ame-level-as-controlling-element

Fix alignment of conditional content beneath radio buttons and checkboxes
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