Skip to content

Conversation

@alexwbbr
Copy link
Collaborator

Updates the common checkbox and radio component to show the conditional when JS is off
#309

@alexwbbr alexwbbr requested a review from daniele-zurico July 21, 2022 09:44
@alexwbbr alexwbbr added this to the 0.5 milestone Jul 21, 2022
@alexwbbr alexwbbr linked an issue Jul 21, 2022 that may be closed by this pull request
@alexwbbr alexwbbr force-pushed the feature/conditional-checkbox-radio branch from de63a80 to e6b10c6 Compare July 22, 2022 13:15
@codecov-commenter
Copy link

codecov-commenter commented Jul 22, 2022

Codecov Report

Merging #325 (8f57a32) into release/0.5 (292fd65) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 8f57a32 differs from pull request most recent head 7c871f2. Consider uploading reports for the commit 7c871f2 to get more accurate results

@@              Coverage Diff              @@
##           release/0.5      #325   +/-   ##
=============================================
  Coverage       100.00%   100.00%           
=============================================
  Files               43        43           
  Lines              955       961    +6     
  Branches           361       361           
=============================================
+ Hits               955       961    +6     
Impacted Files Coverage Δ
src/common/components/CheckboxRadioBase.tsx 100.00% <100.00%> (ø)
src/common/components/Conditional.tsx 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b3e07d...7c871f2. Read the comment docs.

@brightpixels
Copy link
Collaborator

brightpixels commented Jul 22, 2022

@alexwbbr I gave this a test run and I noted 2 issues:

  • With JS disabled, the conditional is displayed, however, with JS enabled, the conditional is missing and toggling the checkbox has no effect.
  • In our use case, we would like the conditional to be a sibiling of the checkbox item and not as a child/nested within the div of the checkbox item. If this will break other use cases, can you give us an option to pass in so we can have the html structure match our requirement, see here: https://design-system.service.gov.uk/components/checkboxes/conditional-reveal/index.html

@alexwbbr
Copy link
Collaborator Author

@brightpixels I have changed the rendering of the conditional which should have fixed the issue where it was no longer being displayed with JS enabled. As for the styling currently there are already lots of classNames you can pass down to the conditional to style it anyway you want which can be defined in the conditional properties section.

package.json Outdated
"author": "Capgemini UK",
"license": "MIT",
"version": "0.5.0-beta-3",
"version": "0.5.0-beta-4",
Copy link
Contributor

Choose a reason for hiding this comment

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

can you please change to beta-5 and update the documentation?

@daniele-zurico daniele-zurico merged commit 9aebce2 into release/0.5 Jul 25, 2022
@daniele-zurico daniele-zurico deleted the feature/conditional-checkbox-radio branch March 14, 2024 11:56
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.

Conditional fields in checkbox only works when JS enabled

5 participants