Skip to content

Conversation

@alan-cole
Copy link
Collaborator

@alan-cole alan-cole commented Oct 21, 2025

Checklist before requesting a review

  • I have formatted the subject to include ticket number as Issue #123456 by drupal_org_username: Issue title
  • I have added a link to the issue tracker
  • I have provided information in Changed section about WHY something was done if this was not a normal implementation
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • I have run new and existing relevant tests locally with my changes, and they passed
  • I have provided screenshots, where applicable

Changed

  1. field.twig was updated to use fieldset > legend instead of label when using checkbox / radio groups. https://github.com/civictheme/uikit/pull/524/files#diff-d0a69dfe038d1ad545b31f1f11b992854ee28789ffa544f1bc34973db2263cfeR109

Screenshots

Screenshot 2025-10-21 at 1 46 59 pm

Summary by CodeRabbit

  • Tests
    • Updated test selectors for checkbox, checkboxes, and radios field tests to reflect changes in form element structure. Assertions now target the appropriate DOM elements for validation checks across multiple test scenarios.

@alan-cole alan-cole self-assigned this Oct 21, 2025
@alan-cole alan-cole added the State: Needs review Pull requests needs a review from assigned developers label Oct 21, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 21, 2025

Walkthrough

DOM selectors in Behat feature tests are updated across checkbox, checkboxes, and radios field scenarios. All references to label elements are systematically replaced with legend elements to align with semantic HTML structure changes in the form fields.

Changes

Cohort / File(s) Summary
Test Selector Updates: Label to Legend
tests/behat/features/styleguide.field.checkbox.feature, tests/behat/features/styleguide.field.checkboxes.feature, tests/behat/features/styleguide.field.radios.feature
Replaced test selectors targeting label elements with legend element equivalents across multiple test scenarios. Changes include selector updates like label:contains(...)legend:contains(...), label.ct-visually-hiddenlegend.ct-visually-hidden, and .ct-field...label.ct-field...legend. Applied consistently to test-checkbox-3, test-checkbox-5, test-checkboxes-1/2/3/5, and test-radios-1/2/3/5 scenarios.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

The changes follow a consistent, repetitive pattern of selector replacements across three related files. Each modification is straightforward (label → legend) with no logic changes or control flow alterations. The homogeneous nature of updates minimizes review complexity, though verification of selector accuracy across all test scenarios is required.

Suggested reviewers

  • richardgaunt

Poem

🐰 From label to legend, a hop and a bound,
Test selectors dancing, new structure found,
Semantic HTML shines ever so bright,
Checkboxes and radios, all set just right! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Updated tests to accomodate changes in uikit/pull/524" accurately reflects the main change in the pull request. The changeset consists of updating test selectors across three Behat feature files to accommodate structural changes from the referenced upstream PR. The title clearly conveys both the action (updating tests) and the context (responding to uikit/pull/524 changes), which aligns with the PR objectives that explain field.twig was updated to use fieldset > legend instead of label. A teammate scanning the repository history would understand that this is a dependency-driven update without needing to review the detailed technical changes.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/uikit-bump-fix

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 90f962a and 523ef52.

📒 Files selected for processing (3)
  • tests/behat/features/styleguide.field.checkbox.feature (2 hunks)
  • tests/behat/features/styleguide.field.checkboxes.feature (4 hunks)
  • tests/behat/features/styleguide.field.radios.feature (4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build-and-push
  • GitHub Check: build-and-push
  • GitHub Check: commit
🔇 Additional comments (6)
tests/behat/features/styleguide.field.checkbox.feature (2)

40-40: LGTM! Semantic HTML improvements properly tested.

The updates correctly migrate field title selectors from label to legend, aligning with the semantic structure for fieldset-based checkbox groups. Line 66 appropriately verifies that legend elements do not have a for attribute, which is semantically correct since legends caption fieldsets rather than associate with specific inputs.

Also applies to: 65-67


52-52: Verify consistency of "hidden title" assertion after label→legend migration.

Test-checkbox-3 (line 40) checks for legend.ct-visually-hidden and test-checkbox-5 (line 65) checks for legend.ct-field__title, but test-checkbox-4 still asserts the absence of label.ct-field__title. If field titles now consistently use legend elements, this should verify the absence of legend.ct-field__title instead. However, if the "Title hidden" scenario uses a different DOM structure without legends, the current assertion may be correct.

tests/behat/features/styleguide.field.checkboxes.feature (2)

13-13: LGTM! Field group titles correctly migrated to legend elements.

The selector updates properly align with semantic HTML structure for checkbox groups, where legend elements caption the fieldset containing multiple checkbox options. Individual option labels (lines 12, 31, 87) correctly remain as label elements.

Also applies to: 32-32, 57-58, 88-88


72-72: Verify consistency of "hidden title" assertion after label→legend migration.

Similar to other test scenarios, visible and visually-hidden titles now use legend elements (lines 13, 32, 57-58, 88), but test-checkboxes-4 still checks for the absence of label.ct__title. Confirm whether the "Title hidden" scenario should verify the absence of legend instead, or if it uses a different DOM structure.

tests/behat/features/styleguide.field.radios.feature (2)

12-13: LGTM! Radio group titles correctly migrated to legend elements.

The updates properly implement semantic HTML structure for radio button groups, with legend elements captioning the fieldset containing radio options. Individual radio option labels (line 87) correctly remain as label elements.

Also applies to: 31-32, 57-58, 88-88


72-72: Verify consistency of "hidden title" assertion after label→legend migration.

Consistent with the pattern across all three test files, visible and visually-hidden titles now use legend elements, but test-radios-4 still checks for the absence of label.ct__title. Confirm whether this assertion should target legend instead, or if the "Title hidden" scenario uses a different structure.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added State: CONFLICT and removed State: Needs review Pull requests needs a review from assigned developers labels Oct 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants