test: add Cypress component tests for all previously unfeat: implement CivicTheme Vue component library with comprehensive test coveragetested components#160
Conversation
Add visual regression and accessibility tests for the 34 civictheme-vue components that had no coverage. Each test uses the existing cy.standardComponentTest() helper, covering all four viewports in both light and dark themes. New tests: Accordion, Alert, BackToTop, Breadcrumb, Checkbox, Collapsible, Flyout, Footer, GroupFilter, Header, Icon, Image, ItemGrid, Label, Link, List, MenuItem, MobileNavigation, NavigationCard, NextStep, Pager, Popover, PromoCard, Radio, Select, SideNavigation, SocialLinks, SubjectCard, TableOfContents, Tabs, Tag, Textarea, Textfield, VideoPlayer.
|
Warning Review limit reached
More reviews will be available in 49 minutes and 1 second. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (39)
📝 WalkthroughWalkthroughThirty-four new Cypress component tests were added covering the entire CivicTheme Vue component library. Each test file follows a consistent pattern: importing a target component, defining a test suite by component category (Atoms, Molecules, Organisms, Base), and mounting via ChangesComponent test coverage across hierarchy
🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (9)
test/components/Flyout.cy.js (2)
12-17: 💤 Low valueDocument why ARIA dialog rules are disabled.
Several ARIA dialog-related accessibility rules are disabled (
aria-required-children,aria-dialog-name,dialog-name). Consider adding a comment explaining why these rules don't apply to the Flyout component (e.g., if it uses a different accessibility pattern than a strict ARIA dialog).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/components/Flyout.cy.js` around lines 12 - 17, The test disables several ARIA dialog axe rules (aria-required-children, aria-dialog-name, dialog-name) inside the axeRules array in Flyout.cy.js without explanation; add a brief comment immediately above the axeRules declaration that documents why these rules are intentionally disabled for the Flyout component (for example: Flyout is not a modal dialog, uses role="presentation"/custom labeling, or accessibility is handled via a different pattern), explicitly listing the disabled rule ids and the rationale so future reviewers know this is intentional and not an oversight.
4-7: Consider more targeted exception handling if possible.The global
Cypress.on('uncaught:exception', () => false)suppresses all uncaught exceptions for this test suite, which could potentially hide real errors unrelated to the querySelector issue. While the comment clearly explains the Twig variable constraint, consider whether the component could be refactored to gracefully handle missing selectors (e.g., with optional chaining or a try-catch block in the source) to allow more targeted exception handling in tests.If the broad suppression is unavoidable due to the Twig-Vue integration pattern, the current approach is acceptable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/components/Flyout.cy.js` around lines 4 - 7, The global suppression masks all errors; either make the component resilient by updating CivicThemeFlyout.findOpenTrigger() to guard the querySelector call (e.g., check that the interpolated id is a valid string/selector or wrap querySelector in a try/catch and return null if invalid) so tests don't need global suppression, or make the test handler targeted by replacing Cypress.on('uncaught:exception', () => false) with a filtered handler that only returns false for the specific querySelector/Twig-variable error message (inspect err.message and only suppress when it matches the unresolved selector pattern), leaving other exceptions to surface.test/components/GroupFilter.cy.js (1)
9-9: 💤 Low valueConsider removing redundant
titleprop.The
titleprop is set to'Filter results by:', which matches the component's default value. Unless explicitly testing default value behavior, this can be omitted.♻️ Remove redundant default
mountOptions: { propsData: { - title: 'Filter results by:', filters: [🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/components/GroupFilter.cy.js` at line 9, Remove the redundant title prop in the GroupFilter component usage within the test (test/components/GroupFilter.cy.js) — the prop value 'Filter results by:' matches the component's default, so delete the title attribute from the rendered GroupFilter in the test (leave it only if you intend to explicitly assert default behavior); look for the GroupFilter test instance and remove the title prop to simplify the test.test/components/Checkbox.cy.js (1)
8-10: 💤 Low valueConsider removing redundant
theme: 'light'prop.The
cy.standardComponentTesthelper already mounts the component without a theme prop by default (light theme). Explicitly passingtheme: 'light'is redundant unless the component behaves differently with explicit vs. implicit light theme.♻️ Simplified propsData
mountOptions: { - propsData: { - theme: 'light', - }, slots: { default: 'Checkbox label', },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/components/Checkbox.cy.js` around lines 8 - 10, The test passes an explicit light theme via propsData (propsData: { theme: 'light' }) which is redundant because cy.standardComponentTest already mounts the Checkbox with the default light theme; remove the theme: 'light' entry from propsData in test/components/Checkbox.cy.js (or only keep it if you need to assert explicit-prop behavior), leaving propsData either empty or containing only non-theme props so cy.standardComponentTest provides the default light theme.test/components/Accordion.cy.js (1)
17-29: 💤 Low valueConsider clarifying the dual use of
background.The test sets
propsData.background: false(component prop) andoptions.background: true(test helper backdrop styling). While technically correct, this naming collision may confuse future maintainers. Consider adding a comment or removing the explicitpropsData.background: falsesince it matches the component's default.♻️ Optional: remove redundant default or add clarifying comment
mountOptions: { propsData: { - background: false, expandAll: true, panels: [Or add a clarifying comment:
mountOptions: { propsData: { + // Component prop: controls internal accordion background styling background: false, expandAll: true, panels: [ { title: 'Accordion title 1', content: 'Lorem ipsum dolor sit amet, consectetur adipisicing elit.', id: 0 }, { title: 'Accordion title 2', content: 'Lorem ipsum dolor sit amet, consectetur adipisicing elit.', id: 1 }, { title: 'Accordion title 3', content: 'Lorem ipsum dolor sit amet, consectetur adipisicing elit.', id: 2 }, ], }, }, options: { + // Test option: applies body background-color for visual testing background: true, },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/components/Accordion.cy.js` around lines 17 - 29, The test uses two different "background" settings—propsData.background on the Accordion component and options.background for the test harness—causing potential confusion; either remove the redundant propsData.background: false (since the component default already sets it) or add a one-line clarifying comment next to propsData.background and options.background explaining that propsData.background is the component prop and options.background controls the test runner/backdrop, referencing the propsData.background and options.background occurrences in the Accordion.cy.js test to locate and update them.test/components/ItemGrid.cy.js (1)
8-11: 💤 Low valueConsider removing redundant
fullprop.The
fullprop is set tofalse, which matches the component's default value. Unless explicitly testing the default behavior, this can be omitted.♻️ Remove redundant default
mountOptions: { propsData: { - full: false, items: ['Item 1', 'Item 2', 'Item 3', 'Item 4'], },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/components/ItemGrid.cy.js` around lines 8 - 11, The test sets the component prop full to false even though that matches the component's default; remove the redundant full: false entry from the propsData object in ItemGrid.cy.js (in the test that mounts ItemGrid) so the test relies on the default prop value, unless you intend to explicitly assert behavior when full is false—if so, add an assertion instead of repeating the default.test/components/MobileNavigation.cy.js (2)
33-34: 💤 Low valueConsider using distinct menu items for top and bottom to improve test coverage.
Both
topMenuItemsandbottomMenuItemsuse the samemenuItemsarray. Using different items would better exercise edge cases and help catch issues specific to each menu position.💡 Suggested enhancement
propsData: { theme: 'light', topMenuItems: menuItems, - bottomMenuItems: menuItems, + bottomMenuItems: [ + { title: 'Bottom Link 1', url: '`#bottom1`' }, + { title: 'Bottom Link 2', url: '`#bottom2`' }, + ], },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/components/MobileNavigation.cy.js` around lines 33 - 34, The test currently passes the same menuItems array to both topMenuItems and bottomMenuItems which reduces coverage; define two distinct arrays (e.g., topMenuItems and bottomMenuItems) with differing entries and use those variables in the test setup where topMenuItems and bottomMenuItems are passed to the component, then update any assertions that check menu contents to account for the differing items so the test exercises position-specific behavior in MobileNavigation.cy.js.
6-6: ⚡ Quick winCTMenuItem.components assignment matches an existing test-suite convention
CTMenuItem.components = { CTLink }does mutate global component state, but the same.components = { ... }wiring approach is used acrosstest/components/(e.g.,Tabs.cy.js,ServiceCard.cy.js,MenuItem.cy.js,Menu.cy.js,Navigation.cy.js). No change needed here; optional consideration is only for stricter isolation if tests were ever run in parallel.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/components/MobileNavigation.cy.js` at line 6, The current test assigns CTMenuItem.components = { CTLink }, which mutates global component state but matches existing test-suite convention; leave it unchanged to maintain consistency, or if you want stricter isolation (e.g., for parallel runs) remove the global mutation and register CTLink locally when mounting the component by passing it via the mount options (e.g., use the test mount call to provide global.components: { CTLink }) instead of modifying CTMenuItem.components.test/components/List.cy.js (1)
14-16: ⚡ Quick winConsider using distinct titles for better test coverage.
All three items use the identical title
'title'. Using distinct values (e.g.,'Title 1','Title 2','Title 3') would better validate that the List component correctly renders different item data, rather than potentially masking bugs where items are duplicated or only the first item renders.✨ Proposed enhancement
items: [ - { title: 'title', imageSrc, id: 0 }, - { title: 'title', imageSrc, id: 1 }, - { title: 'title', imageSrc, id: 2 }, + { title: 'Title 1', imageSrc, id: 0 }, + { title: 'Title 2', imageSrc, id: 1 }, + { title: 'Title 3', imageSrc, id: 2 }, ],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/components/List.cy.js` around lines 14 - 16, The test data for the List spec uses identical titles which can hide rendering bugs; update the items array (the test fixture used when mounting/rendering the List component in List.cy.js) so each item has a unique title (e.g., 'Title 1', 'Title 2', 'Title 3') and keep imageSrc/id as-is; this ensures the List rendering assertions (where the test queries by title/text) validate distinct items rather than duplicated strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/components/Footer.cy.js`:
- Around line 23-26: The test stubs in test/components/Footer.cy.js list
CTNavigation and CTSocialLinks which Footer.vue does not use (it only uses
ThemeMixin and slot-based markup); remove those unused stubs or replace them
with stubs for the actual child components that Footer.vue renders (inspect
Footer.vue to find the real child component names and update the stubs object
accordingly) so the test only stubs relevant components.
In `@test/components/Header.cy.js`:
- Line 14: The test currently disables the color-contrast rule via the object {
id: 'color-contrast', enabled: false' } in Header.cy.js; re-enable it by setting
enabled: true (or remove the override) so the accessibility check runs, then fix
the header CSS (text/background colors) to meet WCAG contrast ratios if the test
fails; if you must keep an exception, add a detailed comment citing the
specific, temporary reason and a TODO ticket/issue reference and ensure the
exception is narrowly scoped to the exact selector or element instead of
globally disabling color-contrast.
- Around line 23-26: The test's stubs object includes CTNavigation and
CTMobileNavigation which are not used by the Header component; update
test/components/Header.cy.js to either remove those unused stubs or replace them
with the actual child components that Header.vue renders (e.g., the ct-header or
whatever named slot children) so stubbing matches the real rendered children;
locate the stubs object in the test and adjust it to only stub components
referenced by Header.vue (or modify the component under test to include the
expected child component names if intentional).
In `@test/components/Icon.cy.js`:
- Line 7: The global Cypress.on('uncaught:exception', () => false) is
suppressing all exceptions; restrict it by either moving the handler inside this
test's describe block (or its before/beforeEach) so it only applies to
Icon.cy.js, and/or change the handler to check the error message/instance (e.g.,
match known benign text) before returning false so only the specific known
exception is swallowed; update the reference to the global handler
(Cypress.on('uncaught:exception', ...)) in Icon.cy.js accordingly.
In `@test/components/Link.cy.js`:
- Around line 8-11: The test is passing wrong props and never exercises
external-link logic; update test(s) in Link.cy.js to use the Link.vue API by
passing propsData: { link: '...' } instead of text/href, and add an extra case
where propsData.link is an external URL (e.g. starting with https://) then
assert the rendered anchor has the ct-link--external class and target="_blank"
(and keep the existing internal-link assertions for the default or internal link
case); reference the Link.vue prop name "link" and the external indicator class
"ct-link--external" when making the changes.
In `@test/components/MenuItem.cy.js`:
- Line 5: Replace the global, unconditional exception suppression
(Cypress.on('uncaught:exception', () => false)) with a scoped or filtered
handler: move the Cypress.on call into the test lifecycle (e.g., register in
beforeEach and remove in afterEach) or change its callback to inspect the error
and return false only for the specific known error messages thrown by
CivicThemeFlyout/collapsible; reference the existing
Cypress.on('uncaught:exception', ...) usage and update test setup to ensure only
the expected error strings are suppressed while other uncaught exceptions still
fail the test.
In `@test/components/Select.cy.js`:
- Around line 16-20: The test disables the axe 'label' rule in the axeRules
array which masks an accessibility issue; instead re-enable the 'label' rule and
update the test's mountOptions to render the component with an accessible label
wrapper (e.g., mount(..., { slots: { default: '<label>My label<CTSelect
.../></label>' } } ) or wrap the rendered Select in a <label> via the test
harness) so the <select> is associated with a label; alternatively, if the
component is intentionally labeled externally, add a short comment in
Select.cy.js near the axeRules/CTSelect mount explaining that labeling is done
at integration level and ensure integration tests cover the labeled usage.
In `@test/components/Tabs.cy.js`:
- Line 4: The global handler Cypress.on('uncaught:exception', () => false) is
too broad; replace it with a scoped or filtered handler so tests don't hide real
bugs: remove the unconditional global handler and either attach a handler inside
the specific test lifecycle (e.g., inside beforeEach/it for the Tabs spec) or
implement a filter in the Cypress.on handler that only returns false for known
expected error signatures (inspect err.message or err.name) and rethrows/lets
other errors surface; update the handler reference to the original Cypress.on
call in Tabs.cy.js and ensure only expected exceptions are suppressed.
---
Nitpick comments:
In `@test/components/Accordion.cy.js`:
- Around line 17-29: The test uses two different "background"
settings—propsData.background on the Accordion component and options.background
for the test harness—causing potential confusion; either remove the redundant
propsData.background: false (since the component default already sets it) or add
a one-line clarifying comment next to propsData.background and
options.background explaining that propsData.background is the component prop
and options.background controls the test runner/backdrop, referencing the
propsData.background and options.background occurrences in the Accordion.cy.js
test to locate and update them.
In `@test/components/Checkbox.cy.js`:
- Around line 8-10: The test passes an explicit light theme via propsData
(propsData: { theme: 'light' }) which is redundant because
cy.standardComponentTest already mounts the Checkbox with the default light
theme; remove the theme: 'light' entry from propsData in
test/components/Checkbox.cy.js (or only keep it if you need to assert
explicit-prop behavior), leaving propsData either empty or containing only
non-theme props so cy.standardComponentTest provides the default light theme.
In `@test/components/Flyout.cy.js`:
- Around line 12-17: The test disables several ARIA dialog axe rules
(aria-required-children, aria-dialog-name, dialog-name) inside the axeRules
array in Flyout.cy.js without explanation; add a brief comment immediately above
the axeRules declaration that documents why these rules are intentionally
disabled for the Flyout component (for example: Flyout is not a modal dialog,
uses role="presentation"/custom labeling, or accessibility is handled via a
different pattern), explicitly listing the disabled rule ids and the rationale
so future reviewers know this is intentional and not an oversight.
- Around line 4-7: The global suppression masks all errors; either make the
component resilient by updating CivicThemeFlyout.findOpenTrigger() to guard the
querySelector call (e.g., check that the interpolated id is a valid
string/selector or wrap querySelector in a try/catch and return null if invalid)
so tests don't need global suppression, or make the test handler targeted by
replacing Cypress.on('uncaught:exception', () => false) with a filtered handler
that only returns false for the specific querySelector/Twig-variable error
message (inspect err.message and only suppress when it matches the unresolved
selector pattern), leaving other exceptions to surface.
In `@test/components/GroupFilter.cy.js`:
- Line 9: Remove the redundant title prop in the GroupFilter component usage
within the test (test/components/GroupFilter.cy.js) — the prop value 'Filter
results by:' matches the component's default, so delete the title attribute from
the rendered GroupFilter in the test (leave it only if you intend to explicitly
assert default behavior); look for the GroupFilter test instance and remove the
title prop to simplify the test.
In `@test/components/ItemGrid.cy.js`:
- Around line 8-11: The test sets the component prop full to false even though
that matches the component's default; remove the redundant full: false entry
from the propsData object in ItemGrid.cy.js (in the test that mounts ItemGrid)
so the test relies on the default prop value, unless you intend to explicitly
assert behavior when full is false—if so, add an assertion instead of repeating
the default.
In `@test/components/List.cy.js`:
- Around line 14-16: The test data for the List spec uses identical titles which
can hide rendering bugs; update the items array (the test fixture used when
mounting/rendering the List component in List.cy.js) so each item has a unique
title (e.g., 'Title 1', 'Title 2', 'Title 3') and keep imageSrc/id as-is; this
ensures the List rendering assertions (where the test queries by title/text)
validate distinct items rather than duplicated strings.
In `@test/components/MobileNavigation.cy.js`:
- Around line 33-34: The test currently passes the same menuItems array to both
topMenuItems and bottomMenuItems which reduces coverage; define two distinct
arrays (e.g., topMenuItems and bottomMenuItems) with differing entries and use
those variables in the test setup where topMenuItems and bottomMenuItems are
passed to the component, then update any assertions that check menu contents to
account for the differing items so the test exercises position-specific behavior
in MobileNavigation.cy.js.
- Line 6: The current test assigns CTMenuItem.components = { CTLink }, which
mutates global component state but matches existing test-suite convention; leave
it unchanged to maintain consistency, or if you want stricter isolation (e.g.,
for parallel runs) remove the global mutation and register CTLink locally when
mounting the component by passing it via the mount options (e.g., use the test
mount call to provide global.components: { CTLink }) instead of modifying
CTMenuItem.components.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 22d90211-2dc3-4db0-963f-eed01ed22432
📒 Files selected for processing (34)
test/components/Accordion.cy.jstest/components/Alert.cy.jstest/components/BackToTop.cy.jstest/components/Breadcrumb.cy.jstest/components/Checkbox.cy.jstest/components/Collapsible.cy.jstest/components/Flyout.cy.jstest/components/Footer.cy.jstest/components/GroupFilter.cy.jstest/components/Header.cy.jstest/components/Icon.cy.jstest/components/Image.cy.jstest/components/ItemGrid.cy.jstest/components/Label.cy.jstest/components/Link.cy.jstest/components/List.cy.jstest/components/MenuItem.cy.jstest/components/MobileNavigation.cy.jstest/components/NavigationCard.cy.jstest/components/NextStep.cy.jstest/components/Pager.cy.jstest/components/Popover.cy.jstest/components/PromoCard.cy.jstest/components/Radio.cy.jstest/components/Select.cy.jstest/components/SideNavigation.cy.jstest/components/SocialLinks.cy.jstest/components/SubjectCard.cy.jstest/components/TableOfContents.cy.jstest/components/Tabs.cy.jstest/components/Tag.cy.jstest/components/Textarea.cy.jstest/components/Textfield.cy.jstest/components/VideoPlayer.cy.js
| stubs: { | ||
| CTNavigation: true, | ||
| CTSocialLinks: true, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check component registration in Footer.vue
rg -A 5 'components:\s*\{' src/components/Footer.vueRepository: Realityloop/civictheme-vue
Length of output: 52
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate Footer.vue =="
ls -la src/components/Footer.vue 2>/dev/null || true
fd -a -t f 'Footer.vue' src 2>/dev/null || true
echo
echo "== Inspect test stubs in Footer.cy.js (lines ~15-40) =="
sed -n '1,120p' test/components/Footer.cy.js | nl -ba | sed -n '15,60p' || true
echo
echo "== Search for CTNavigation / CTSocialLinks in Footer.vue =="
fd -t f 'Footer.vue' src | head -n 20 | xargs -I{} sh -c 'echo "--- {} ---"; rg -n "CTNavigation|CTSocialLinks" "{}" || true'
echo
echo "== Show how components are registered in Footer.vue (near any components/defineComponent/registration code) =="
fd -t f 'Footer.vue' src | head -n 20 | xargs -I{} sh -c 'echo "--- {} ---"; rg -n "components\s*:|defineComponent|components\s*\{|register|CTNavigation|CTSocialLinks" "{}" || true;'
echo
echo "== Dump Footer.vue (truncated) for manual registration context =="
fd -t f 'Footer.vue' src | head -n 20 | while read -r f; do echo "--- $f (first 220 lines) ---"; head -n 220 "$f"; doneRepository: Realityloop/civictheme-vue
Length of output: 3428
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Footer.cy.js =="
ls -la test/components/Footer.cy.js 2>/dev/null || true
sed -n '1,200p' test/components/Footer.cy.js | cat -n
echo
echo "== Search CTNavigation / CTSocialLinks usage =="
rg -n "CTNavigation|CTSocialLinks" src test || trueRepository: Realityloop/civictheme-vue
Length of output: 3267
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Match in Footer.vue =="
rg -n "CTNavigation|CTSocialLinks" src/components/Footer.vue || true
echo
echo "== Inspect ThemeMixin used by Footer.vue =="
ls -la src/mixins/theme* 2>/dev/null || true
rg -n "CTNavigation|CTSocialLinks" src/mixins/theme* src/mixins/** 2>/dev/null || true
sed -n '1,200p' src/mixins/theme* 2>/dev/null || trueRepository: Realityloop/civictheme-vue
Length of output: 376
Remove/adjust unused stubs in test/components/Footer.cy.js
src/components/Footer.vue contains only ThemeMixin and slot-based markup; it does not reference CTNavigation or CTSocialLinks. The stubs entries for those components therefore won’t affect the test—either remove them or stub the actual child components Footer.vue uses.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/components/Footer.cy.js` around lines 23 - 26, The test stubs in
test/components/Footer.cy.js list CTNavigation and CTSocialLinks which
Footer.vue does not use (it only uses ThemeMixin and slot-based markup); remove
those unused stubs or replace them with stubs for the actual child components
that Footer.vue renders (inspect Footer.vue to find the real child component
names and update the stubs object accordingly) so the test only stubs relevant
components.
| { id: 'region', enabled: false }, | ||
| { id: 'landmark-one-main', enabled: false }, | ||
| { id: 'landmark-banner-is-top-level', enabled: false }, | ||
| { id: 'color-contrast', enabled: false }, |
There was a problem hiding this comment.
Disabling color-contrast accessibility check may hide real issues.
Unlike the landmark and structural rules disabled above, color-contrast checks for actual visual accessibility problems that affect users. Disabling it means the test won't catch insufficient contrast ratios that could make the header difficult to read for users with visual impairments.
Consider whether the header truly requires this exception, or if the underlying contrast issues should be fixed instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/components/Header.cy.js` at line 14, The test currently disables the
color-contrast rule via the object { id: 'color-contrast', enabled: false' } in
Header.cy.js; re-enable it by setting enabled: true (or remove the override) so
the accessibility check runs, then fix the header CSS (text/background colors)
to meet WCAG contrast ratios if the test fails; if you must keep an exception,
add a detailed comment citing the specific, temporary reason and a TODO
ticket/issue reference and ensure the exception is narrowly scoped to the exact
selector or element instead of globally disabling color-contrast.
| stubs: { | ||
| CTNavigation: true, | ||
| CTMobileNavigation: true, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check component registration in Header.vue
rg -A 5 'components:\s*\{' src/components/Header.vueRepository: Realityloop/civictheme-vue
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Locate Header.vue
fd -a "Header.vue" -t f .
# Search for the stubbed component names in the real Header.vue
HEADER_FILE="$(fd -a "Header.vue" -t f . | head -n 1)"
echo "Using: $HEADER_FILE"
rg -n "CTNavigation|CTMobileNavigation" "$HEADER_FILE" -S
# If Header.vue is options API, also inspect any components/registration blocks
rg -n "components\s*:" "$HEADER_FILE" -SRepository: Realityloop/civictheme-vue
Length of output: 268
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate both Header.vue files
FILES=$(fd -a "Header.vue" -t f .)
echo "$FILES"
# Show the stub snippet in the test for context
echo "---- test/components/Header.cy.js (lines 1-120) ----"
nl -ba test/components/Header.cy.js | sed -n '1,120p'
for f in $FILES; do
echo "---- $f : CTNavigation / CTMobileNavigation matches ----"
rg -n "CTNavigation|CTMobileNavigation" "$f" -S || true
echo "---- $f : nearby component registration candidates ----"
# Look for Vue component registration patterns
rg -n "components\s*:" "$f" -S || true
rg -n "import\s+.*CTNavigation|import\s+.*CTMobileNavigation" "$f" -S || true
rg -n "resolveComponent|defineAsyncComponent|defineComponent" "$f" -S || true
# Print a reasonable excerpt around the script section (helps determine import/aliasing)
echo "---- $f : excerpt around <script ----"
rg -n "<script" "$f" -n -S || true
# If there is <script setup> or script block, print first ~120 lines after it
# (Use sed range based on first occurrence)
start=$(rg -n "<script" "$f" -S | head -n1 | cut -d: -f1 || true)
if [ -n "${start:-}" ]; then
s=$((start))
e=$((s+140))
nl -ba "$f" | sed -n "${s},${e}p"
fi
doneRepository: Realityloop/civictheme-vue
Length of output: 303
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate both Header.vue files
FILES=$(fd -a "Header.vue" -t f .)
echo "Found Header.vue files:"
echo "$FILES"
echo "---- test/components/Header.cy.js (lines 15-60) ----"
# Use cat -n (nl may be unavailable)
cat -n test/components/Header.cy.js | sed -n '15,60p' || true
for f in $FILES; do
echo "==== $f : CTNavigation / CTMobileNavigation occurrences ===="
rg -n "CTNavigation|CTMobileNavigation" "$f" -S || true
echo "==== $f : likely registration / imports ===="
rg -n "components\s*:" "$f" -S || true
rg -n "import\s+.*CTNavigation|import\s+.*CTMobileNavigation" "$f" -S || true
rg -n "resolveComponent|defineAsyncComponent|defineComponent" "$f" -S || true
echo "==== $f : excerpt around <script> tags ===="
# print up to ~180 lines after first <script occurrences
first_script=$(rg -n "<script" "$f" -S | head -n1 | cut -d: -f1 || true)
if [ -n "${first_script:-}" ]; then
s=$first_script
e=$((s+180))
cat -n "$f" | sed -n "${s},${e}p" | head -n 220
else
echo "(no <script> tag found)"
fi
doneRepository: Realityloop/civictheme-vue
Length of output: 3511
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "==== Search for CTNavigation usage in Header.vue files ===="
FILES=$(fd -a "Header.vue" -t f .)
for f in $FILES; do
echo "--- $f ---"
rg -n "CTNavigation|CTMobileNavigation" "$f" -S || true
done
SRC_HEADER="/home/jailuser/git/src/components/Header.vue"
NUXT_HEADER="/home/jailuser/git/example/nuxt/components/druxt/block-region/Header.vue"
echo "==== Excerpt: src/components/Header.vue template (around likely component tags) ===="
# Print from <template> through first ~200 lines
tpl=$(rg -n "<template" "$SRC_HEADER" -S | head -n1 | cut -d: -f1 || true)
if [ -n "${tpl:-}" ]; then
s=$tpl
e=$((tpl+220))
cat -n "$SRC_HEADER" | sed -n "${s},${e}p"
else
echo "(no <template> found)"
fi
echo "==== Excerpt: nuxt/components/druxt/block-region/Header.vue (full file) ===="
cat -n "$NUXT_HEADER" | head -n 220
echo "==== Search and excerpt in test/components/Header.cy.js for CTNavigation stubs context ===="
rg -n "CTNavigation|CTMobileNavigation" test/components/Header.cy.js -S || true
cat -n test/components/Header.cy.js | sed -n '1,140p'Repository: Realityloop/civictheme-vue
Length of output: 7269
Remove/adjust unused CTNavigation stubs in Header tests
src/components/Header.vue renders only ct-header markup with named slots and does not reference CTNavigation or CTMobileNavigation, so the stubs entries in test/components/Header.cy.js are unused. Either remove them or update the test/component so it stubs the actual child components being rendered.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/components/Header.cy.js` around lines 23 - 26, The test's stubs object
includes CTNavigation and CTMobileNavigation which are not used by the Header
component; update test/components/Header.cy.js to either remove those unused
stubs or replace them with the actual child components that Header.vue renders
(e.g., the ct-header or whatever named slot children) so stubbing matches the
real rendered children; locate the stubs object in the test and adjust it to
only stub components referenced by Header.vue (or modify the component under
test to include the expected child component names if intentional).
| // Icon.vue's svg computed returns () => import('...svg?inline'). In the | ||
| // component test bundler the import resolves to undefined, causing a | ||
| // TypeError when Vue tries to extract props from the result. | ||
| Cypress.on('uncaught:exception', () => false) |
There was a problem hiding this comment.
Scope exception suppression to avoid masking real bugs.
The global Cypress.on('uncaught:exception', () => false) suppresses all uncaught exceptions for the entire Cypress process, not just this component test. This can hide legitimate bugs in Icon, other components, or test infrastructure.
🔒 Recommended fix: filter by error type or move inside test
Option 1: Filter by specific error message
-Cypress.on('uncaught:exception', () => false)
+Cypress.on('uncaught:exception', (err) => {
+ // Suppress only the specific SVG import TypeError
+ if (err.message.includes('Cannot read properties of undefined')) {
+ return false
+ }
+ // Let other exceptions fail the test
+ return true
+})Option 2: Scope to this describe block
describe('Atoms/Icon', () => {
+ before(() => {
+ Cypress.on('uncaught:exception', () => false)
+ })
+
it('Default', () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Cypress.on('uncaught:exception', () => false) | |
| Cypress.on('uncaught:exception', (err) => { | |
| // Suppress only the specific SVG import TypeError | |
| if (err.message.includes('Cannot read properties of undefined')) { | |
| return false | |
| } | |
| // Let other exceptions fail the test | |
| return true | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/components/Icon.cy.js` at line 7, The global
Cypress.on('uncaught:exception', () => false) is suppressing all exceptions;
restrict it by either moving the handler inside this test's describe block (or
its before/beforeEach) so it only applies to Icon.cy.js, and/or change the
handler to check the error message/instance (e.g., match known benign text)
before returning false so only the specific known exception is swallowed; update
the reference to the global handler (Cypress.on('uncaught:exception', ...)) in
Icon.cy.js accordingly.
| propsData: { | ||
| theme: 'light', | ||
| text: 'Link text', | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if Link component requires href or url prop
rg -A 10 'props:\s*\{' src/components/Link.vue | head -30Repository: Realityloop/civictheme-vue
Length of output: 251
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Inspect Link.vue props ==="
rg -n "props\s*:" src/components/Link.vue
rg -n "href|url" src/components/Link.vue
echo
echo "=== Show Link.vue props block ==="
# Print a larger window around the props block
rg -n "props\s*:\s*\{" -n -S src/components/Link.vue | head -20
# If multiple matches, show around first occurrence
line=$(rg -n "props\s*:\s*\{" -S src/components/Link.vue | head -1 | cut -d: -f1)
start=$((line))
end=$((line+120))
sed -n "${start},${end}p" src/components/Link.vue
echo
echo "=== Show template anchor usage ==="
rg -n "<a\\b|<router-link\\b" -S src/components/Link.vue
# Show likely section around first <a or router-link
tmpl_line=$(rg -n "<a\\b|<router-link\\b" -S src/components/Link.vue | head -1 | cut -d: -f1)
start2=$((tmpl_line-40))
end2=$((tmpl_line+80))
sed -n "${start2},${end2}p" src/components/Link.vueRepository: Realityloop/civictheme-vue
Length of output: 1661
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Link.cy.js ==="
sed -n '1,160p' test/components/Link.cy.js
echo
echo "=== Search for 'link' usage in Link.cy.js ==="
rg -n "link|href|url|external|NuxtLink|ct-link" test/components/Link.cy.js || trueRepository: Realityloop/civictheme-vue
Length of output: 492
Adjust Link.cy.js props to match Link.vue API and cover external detection
src/components/Link.vue uses a link prop (default '#') to render the anchor (href: link / to: link), so the test won’t fail due to missing href/url. However, with the default link='#', isExternal stays false, so the external-link detection path isn’t exercised—add a case with link set to an external URL (and assert ct-link--external / target).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/components/Link.cy.js` around lines 8 - 11, The test is passing wrong
props and never exercises external-link logic; update test(s) in Link.cy.js to
use the Link.vue API by passing propsData: { link: '...' } instead of text/href,
and add an extra case where propsData.link is an external URL (e.g. starting
with https://) then assert the rendered anchor has the ct-link--external class
and target="_blank" (and keep the existing internal-link assertions for the
default or internal link case); reference the Link.vue prop name "link" and the
external indicator class "ct-link--external" when making the changes.
| import CTLink from '../../src/components/Link.vue' | ||
|
|
||
| // MenuItem initialises CivicThemeFlyout and collapsible in mounted(). | ||
| Cypress.on('uncaught:exception', () => false) |
There was a problem hiding this comment.
Global exception suppression is too broad and could hide bugs.
Line 5 uses Cypress.on('uncaught:exception', () => false) to suppress ALL uncaught exceptions within this spec file. While the comment explains that MenuItem's mounted() lifecycle initializes CivicThemeFlyout and collapsible (which may throw in the test environment), unconditionally returning false will also hide legitimate bugs or errors in the component logic.
This pattern reduces test effectiveness because runtime errors that should fail the test will be silently ignored, potentially allowing regressions to pass undetected.
🛡️ Proposed fix: Scope exception handling or filter specific errors
Option 1: Scope to the test lifecycle
-Cypress.on('uncaught:exception', () => false)
import CTMenuItem from '../../src/components/MenuItem.vue'
CTMenuItem.components = { CTLink }
import '`@civictheme/uikit/components/00-base/collapsible/collapsible`'
describe('Molecules/MenuItem', () => {
+ beforeEach(() => {
+ cy.on('uncaught:exception', () => false)
+ })
+
it('Default', () => {Option 2 (preferred): Filter for specific known errors
-Cypress.on('uncaught:exception', () => false)
+Cypress.on('uncaught:exception', (err) => {
+ // Ignore known initialization errors from CivicTheme collapsible/flyout in test env
+ if (err.message.includes('collapsible') || err.message.includes('flyout')) {
+ return false
+ }
+ // Allow other exceptions to fail the test
+ return true
+})
import CTMenuItem from '../../src/components/MenuItem.vue'This ensures only expected errors are suppressed while legitimate bugs still cause test failures.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Cypress.on('uncaught:exception', () => false) | |
| Cypress.on('uncaught:exception', (err) => { | |
| // Ignore known initialization errors from CivicTheme collapsible/flyout in test env | |
| if (err.message.includes('collapsible') || err.message.includes('flyout')) { | |
| return false | |
| } | |
| // Allow other exceptions to fail the test | |
| return true | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/components/MenuItem.cy.js` at line 5, Replace the global, unconditional
exception suppression (Cypress.on('uncaught:exception', () => false)) with a
scoped or filtered handler: move the Cypress.on call into the test lifecycle
(e.g., register in beforeEach and remove in afterEach) or change its callback to
inspect the error and return false only for the specific known error messages
thrown by CivicThemeFlyout/collapsible; reference the existing
Cypress.on('uncaught:exception', ...) usage and update test setup to ensure only
the expected error strings are suppressed while other uncaught exceptions still
fail the test.
| axeRules: [ | ||
| { id: 'page-has-heading-one', enabled: false }, | ||
| { id: 'label', enabled: false }, | ||
| { id: 'select-name', enabled: false }, | ||
| ], |
There was a problem hiding this comment.
Disabling the 'label' accessibility rule tests the component in an inaccessible state.
Line 18 disables the axe 'label' rule, which checks that form inputs have accessible labels. While this allows the isolated component test to pass, it validates a non-accessible pattern where a <select> element renders without an associated label. In real-world usage, this would fail WCAG 2.1 Level A compliance (SC 3.3.2 Labels or Instructions).
Consider one of these approaches:
- Wrap the component in a proper label structure within
mountOptionsto test the accessible pattern - Document explicitly why the component is designed to be labeled externally and ensure integration tests validate proper labeling
♿ Proposed fix: Test with accessible label wrapper
describe('Atoms/Select', () => {
it('Default', () => {
cy.standardComponentTest(CTSelect, {
mountOptions: {
propsData: {
+ name: 'test-select',
options: [
{ value: 1, label: 'Option 1' },
{ value: 2, label: 'Option 2' },
{ value: 3, label: 'Option 3' },
],
},
+ slots: {
+ default: '<label for="test-select">Select an option</label>'
+ }
},
axeRules: [
{ id: 'page-has-heading-one', enabled: false },
- { id: 'label', enabled: false },
{ id: 'select-name', enabled: false },
],
})
})
})Note: Adjust slot/wrapper pattern to match your test setup if CTSelect doesn't support this structure directly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/components/Select.cy.js` around lines 16 - 20, The test disables the axe
'label' rule in the axeRules array which masks an accessibility issue; instead
re-enable the 'label' rule and update the test's mountOptions to render the
component with an accessible label wrapper (e.g., mount(..., { slots: { default:
'<label>My label<CTSelect .../></label>' } } ) or wrap the rendered Select in a
<label> via the test harness) so the <select> is associated with a label;
alternatively, if the component is intentionally labeled externally, add a short
comment in Select.cy.js near the axeRules/CTSelect mount explaining that
labeling is done at integration level and ensure integration tests cover the
labeled usage.
| /* global cy, describe, it, Cypress */ | ||
| import CTLink from '../../src/components/Link.vue' | ||
|
|
||
| Cypress.on('uncaught:exception', () => false) |
There was a problem hiding this comment.
Global exception suppression is too broad and could hide bugs.
Line 4 uses Cypress.on('uncaught:exception', () => false) to unconditionally suppress ALL uncaught exceptions within this spec file. While this may be intended to handle errors from component initialization, it will also hide legitimate runtime errors or bugs in the Tabs component logic, reducing the test's ability to detect regressions.
🛡️ Proposed fix: Scope exception handling or filter specific errors
Option 1: Scope to the test lifecycle
-Cypress.on('uncaught:exception', () => false)
import CTItemList from '../../src/components/ItemList.vue'
import CTTabs from '../../src/components/Tabs.vue'
CTTabs.components = { CTItemList, CTLink }
describe('Molecules/Tabs', () => {
+ beforeEach(() => {
+ cy.on('uncaught:exception', () => false)
+ })
+
it('Default', () => {Option 2 (preferred): Filter for specific known errors
-Cypress.on('uncaught:exception', () => false)
+Cypress.on('uncaught:exception', (err) => {
+ // Ignore known initialization errors in test environment
+ // Update filter condition based on actual error messages observed
+ if (err.message.includes('specific-expected-error-keyword')) {
+ return false
+ }
+ // Allow other exceptions to fail the test
+ return true
+})
import CTItemList from '../../src/components/ItemList.vue'This ensures only expected errors are suppressed while legitimate bugs still cause test failures. If no specific errors are expected, consider removing the handler entirely and investigating why exceptions occur during component initialization.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Cypress.on('uncaught:exception', () => false) | |
| Cypress.on('uncaught:exception', (err) => { | |
| // Ignore known initialization errors in test environment | |
| // Update filter condition based on actual error messages observed | |
| if (err.message.includes('specific-expected-error-keyword')) { | |
| return false | |
| } | |
| // Allow other exceptions to fail the test | |
| return true | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/components/Tabs.cy.js` at line 4, The global handler
Cypress.on('uncaught:exception', () => false) is too broad; replace it with a
scoped or filtered handler so tests don't hide real bugs: remove the
unconditional global handler and either attach a handler inside the specific
test lifecycle (e.g., inside beforeEach/it for the Tabs spec) or implement a
filter in the Cypress.on handler that only returns false for known expected
error signatures (inspect err.message or err.name) and rethrows/lets other
errors surface; update the handler reference to the original Cypress.on call in
Tabs.cy.js and ensure only expected exceptions are suppressed.
1 participant
Types of changes
Description
Implements the full CivicTheme Vue component library with new components, enhancements to existing ones, Storybook stories, and Cypress component tests.
New Components
Enhancements
@civictheme/uikitIcon.vuecomponentLink.vuewith external link detection, icon prop, and class passthroughList,GroupFilter, and accordion panelPopoverTag,Textfield, and form elementsTabsTesting
Checklist:
Screenshots/Media:
Summary by CodeRabbit
Release Notes