Sc 12778/improve components to be wcag 2.2 aa compliant #155
Conversation
… searchField & implement live-region for autocomplete
…se role, state, and properties & Current values exposed for inputs
WalkthroughAdds broad accessibility and semantic markup changes across UI components, introduces live-region announcement logic in Autocomplete and SearchResults, synchronizes ARIA state in SearchField, refines event handling in ActiveFilters, registers a Handlebars Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Input as SearchField (combobox)
participant AC as Autocomplete
participant LR as LiveRegion (status)
User->>Input: Type / navigate
Input->>AC: onAutocompleteUpdate(state)
AC->>AC: render suggestions
AC->>LR: updateLiveRegion(totalSuggestions)
Input->>Input: updateAriaAttributes(state)
Input->>Input: updateAriaAttributesForBoundFields(state)
sequenceDiagram
autonumber
participant Renderer as SearchResults
participant LR as LiveRegion (status)
Renderer->>Renderer: render(container, data)
Renderer->>LR: updateLiveRegion(totalHits, keyword) (dedupe via lastAnnouncedResults)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/aianswersresult/precompile-templates/aianswersresult.handlebars (1)
146-160: Remove redundant new tab announcement.The source links currently include both an
aria-labelwith “(opens in new tab)” and a sr-only<span>with the same text. Remove one to avoid double announcements. Prefer Option 2 (keep the sr-only span):- <a href='{{url}}' class='source' target="_blank" aria-label="{{title}} (opens in new tab)"> + <a href='{{url}}' class='source' target="_blank"> {{title}} <span class="addsearch-sr-only"> (opens in new tab)</span>Show-more button’s
aria-expandedis toggled correctly in the JavaScript.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (32)
README.md(1 hunks)src/components/activefilters/activefilters.js(1 hunks)src/components/activefilters/activefilters.scss(1 hunks)src/components/activefilters/precompile-templates/activefilters.handlebars(1 hunks)src/components/aianswersresult/aianswersresult.js(4 hunks)src/components/aianswersresult/precompile-templates/aianswersresult.handlebars(9 hunks)src/components/autocomplete/autocomplete.js(4 hunks)src/components/autocomplete/precompile-templates/autocomplete.handlebars(1 hunks)src/components/filters/precompile-templates/filters_checkboxgroup.handlebars(1 hunks)src/components/filters/precompile-templates/filters_radiogroup.handlebars(1 hunks)src/components/filters/precompile-templates/filters_range.handlebars(1 hunks)src/components/filters/precompile-templates/filters_selectlist.handlebars(1 hunks)src/components/filters/precompile-templates/filters_tabs.handlebars(1 hunks)src/components/filters/precompile-templates/filters_tags.handlebars(1 hunks)src/components/hierarchicalfacets/hierarchicalfacets.js(1 hunks)src/components/hierarchicalfacets/hierarchicalfacets.scss(3 hunks)src/components/hierarchicalfacets/precompile-templates/hierarchical_facets.handlebars(1 hunks)src/components/hierarchicalfacets/precompile-templates/subHierarchicalFacetsTemplate.handlebars(1 hunks)src/components/pagination/precompile-templates/pagination.handlebars(2 hunks)src/components/rangefacets/precompile-templates/sliderrangefacets.handlebars(1 hunks)src/components/searchfield/precompile-templates/searchfield.handlebars(1 hunks)src/components/searchfield/searchfield.js(3 hunks)src/components/searchresults/precompile-templates/no_results.handlebars(1 hunks)src/components/searchresults/precompile-templates/numberOfResultsTemplate.handlebars(1 hunks)src/components/searchresults/precompile-templates/searchresults.handlebars(1 hunks)src/components/searchresults/searchresults.js(3 hunks)src/components/searchresults/searchresults.scss(1 hunks)src/components/sortby/precompile-templates/sortby_radiogroup.handlebars(1 hunks)src/components/sortby/precompile-templates/sortby_select.handlebars(1 hunks)src/components/sortby/sortby.scss(1 hunks)src/index.scss(1 hunks)src/util/handlebars.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/searchresults/searchresults.js (3)
src/actions/search.js (1)
hits(99-99)src/util/history.js (1)
keyword(237-237)src/reducers/keyword.js (1)
keyword(11-29)
🔇 Additional comments (55)
src/util/handlebars.js (1)
76-78: Add type validation toaddhelper and confirm its usage
I didn’t find any{{addcalls in .hbs/.handlebars templates—please verify where this helper is used and, if it’s used for numeric output, coerce inputs to numbers and return 0 on NaN:registerHelper('add', (a, b) => { - return a + b; + const numA = Number(a); + const numB = Number(b); + if (isNaN(numA) || isNaN(numB)) return 0; + return numA + numB; });src/components/pagination/precompile-templates/pagination.handlebars (1)
1-13: Good semantic HTML and accessibility improvements!The change from
<div>to<nav>is semantically correct for pagination controls, and thearia-label="Search results pagination"appropriately distinguishes this navigation landmark from others on the page. The existing ARIA attributes on buttons (lines 3, 6, 11) and thearia-current="true"for the active page (line 6) are well-implemented.src/components/activefilters/activefilters.scss (1)
9-19: LGTM! Proper styling for semantic list structure.The list style resets and flex properties correctly support the semantic
<ul>structure introduced in the template while maintaining the existing layout behavior.src/components/activefilters/activefilters.js (1)
164-173: Excellent fix for safer event handling.Using
e.currentTargetinstead ofe.targetis the correct approach here. Since the template includes nested<span>elements within buttons,e.targetcould reference the child span when clicked, potentially missing the required data attributes.e.currentTargetreliably references the button element that owns the event listener.src/components/activefilters/precompile-templates/activefilters.handlebars (4)
7-16: Excellent ARIA implementation for accessible remove buttons.The combination of
aria-labelon the button andaria-hidden="true"on the decorative × symbol follows WCAG best practices. The label provides clear context ("Remove [filter name] filter") while hiding the visual-only symbol from screen readers.
22-22: Good accessible label for the clear all button.The
aria-label="Clear all active filters"provides clear context for screen reader users about the button's purpose.
20-24:gthelper registration confirmed Thegthelper is defined insrc/util/handlebars.jsviaregisterHelper('gt', …), so this template usage is valid.
2-2:.addsearch-sr-onlyis correctly defined with all required screen-reader-only styles in src/index.scss (lines 16–24); no changes needed.src/components/filters/precompile-templates/filters_selectlist.handlebars (1)
2-2: LGTM! Accessibility improvement.The
aria-labelprovides context for screen reader users when the select doesn't have a visible label element.README.md (1)
511-514: LGTM! Documentation is clear.The table formatting adjustment and the new
labeloption documentation align with the accessibility improvements being made to filter groups.src/components/filters/precompile-templates/filters_tabs.handlebars (1)
2-4: LGTM! Correct ARIA tabs implementation.The tablist/tab roles with
aria-selectedproperly implement the ARIA tabs pattern, making the filter tabs accessible to assistive technologies.src/components/searchfield/searchfield.js (3)
143-171: LGTM! Consistent ARIA management for bound fields.The method correctly applies the same ARIA attribute updates to all bound input fields, ensuring consistent combobox behavior across multiple search fields.
296-300: LGTM! Proper ARIA initialization.The combobox ARIA attributes are correctly initialized on bound fields, establishing the foundation for the combobox pattern before dynamic updates occur.
99-122: Good ARIA combobox implementation. ID patterns verified. The autocomplete templates include matching IDs (addsearch-suggestion-{{@index}},addsearch-customfield-{{@index}}).src/components/searchfield/precompile-templates/searchfield.handlebars (1)
4-13: LGTM! Complete combobox ARIA attributes.The input element now has all required ARIA attributes for the combobox pattern, matching the JavaScript implementation that manages these attributes dynamically.
src/index.scss (1)
15-26: LGTM! Standard screen-reader-only utility.The
.addsearch-sr-onlyclass correctly implements the standard pattern for visually hiding content while keeping it accessible to screen readers.src/components/sortby/precompile-templates/sortby_select.handlebars (1)
2-2: LGTM! Clear and descriptive label.The
aria-labelprovides clear context about the select's purpose for screen reader users.src/components/searchresults/searchresults.scss (1)
5-10: Approve semantic<ol>reset: Confirmed<ol>usage in searchresults template; list styling reset is appropriate.src/components/hierarchicalfacets/precompile-templates/hierarchical_facets.handlebars (1)
1-3: LGTM! Correct tree role and level tracking.The accessibility implementation is sound:
role="tree"is the appropriate ARIA role for hierarchical navigationaria-labelprovides context for the hierarchical filter structure- Passing
level=1ensures properaria-leveltracking for nested tree items in the child templatesrc/components/searchresults/precompile-templates/numberOfResultsTemplate.handlebars (1)
1-4: LGTM! Proper live region for result count updates.The live region configuration is appropriate:
role="status"correctly identifies this as a status messagearia-live="polite"ensures screen readers announce updates without interrupting the usersrc/components/searchresults/precompile-templates/no_results.handlebars (1)
2-2: LGTM! Properly configured screen-reader-only live region.The live region implementation follows best practices:
role="status"andaria-live="polite"enable non-disruptive announcementsaria-atomic="true"ensures the entire announcement is read when updatedaddsearch-sr-onlyclass hides the region visually while keeping it accessible to screen readersThis pattern allows JavaScript to dynamically announce status updates without visual clutter.
src/components/sortby/precompile-templates/sortby_radiogroup.handlebars (1)
1-7: LGTM! Proper radiogroup semantics and explicit values.The accessibility enhancements are well-implemented:
role="radiogroup"correctly identifies the group of related radio buttonsaria-labelprovides clear context for the sort options- Explicit
valueattribute improves form semantics and programmatic accesssrc/components/aianswersresult/aianswersresult.js (4)
95-95: LGTM! Correct aria-expanded management in initial expanded state.The
aria-expanded="true"attribute correctly reflects the expanded state of the show-more button during initial render when the answer is expanded.
105-105: LGTM! Correct aria-expanded management for overflow content.The
aria-expanded="false"attribute correctly reflects the collapsed state when content overflows and the show-more button is displayed.
122-122: LGTM! Proper aria-expanded update when collapsing.The attribute is correctly updated to
"false"when transitioning from expanded to collapsed state.
134-134: LGTM—aria-expandedupdates correctly and the show-more button already includesaria-controls="ai-answer-content"to link it to the content.src/components/filters/precompile-templates/filters_checkboxgroup.handlebars (1)
1-2: LGTM! Proper group semantics with conditional labeling.The accessibility implementation is correct:
role="group"appropriately groups related checkboxesaria-labelledbycorrectly references the visible label when present- ID pattern
checkbox-label-{{containerId}}ensures uniqueness- Conditional rendering (
{{#if label}}) maintains consistency between the label element and ARIA referencesrc/components/filters/precompile-templates/filters_radiogroup.handlebars (1)
1-2: LGTM! Proper radiogroup semantics with conditional labeling.The accessibility implementation follows best practices:
role="radiogroup"correctly identifies the group of related radio buttonsaria-labelledbyappropriately references the visible label when present- ID pattern
radio-label-{{containerId}}ensures uniqueness across multiple instances- Conditional rendering maintains consistency between the label element and ARIA reference
src/components/sortby/sortby.scss (2)
22-37: LGTM: Consistent focus indicators.The select element focus indicators match the input styling, providing a consistent accessibility experience. The same contrast verification from the previous comment applies here.
14-19: Focus indicator contrast is sufficient
#0066ccyields contrast ratios of 5.57:1 on white and 4.14:1 on #dedede, exceeding the WCAG 2.2 AA minimum of 3:1.src/components/rangefacets/precompile-templates/sliderrangefacets.handlebars (1)
1-2: LGTM: Proper ARIA labeling.The group role with aria-labelledby provides appropriate semantic structure and labeling for the slider control. The screen-reader-only label ensures accessibility without visual clutter.
src/components/hierarchicalfacets/hierarchicalfacets.js (1)
150-162: LGTM: Correct ARIA state synchronization.The ARIA state management is properly implemented:
aria-expandedcorrectly reflects the current state after the togglearia-labelappropriately describes the next action (e.g., "Collapse" when expanded, "Expand" when collapsed)- The DOM manipulation is clean with the
facetWrapvariableThis follows WCAG best practices for accessible expandable/collapsible controls.
src/components/searchresults/searchresults.js (1)
106-107: LGTM: Proper live region integration.The
updateLiveRegioncall is correctly placed after rendering the HTML, ensuring the live region element exists in the DOM before updating it. The deduplication logic prevents unnecessary announcements on re-renders.src/components/hierarchicalfacets/precompile-templates/subHierarchicalFacetsTemplate.handlebars (3)
1-1: LGTM: Proper tree semantics.The treeitem role with aria-level, aria-expanded, and aria-owns correctly implements ARIA tree semantics for hierarchical navigation. The conditional aria-owns ensures the attribute is only present when children exist.
17-19: LGTM: correct ARIA group semantics and recursion
role="group"usage and level increment via theaddhelper are correct; helper is registered insrc/util/handlebars.js.
4-10: Verify keyboard accessibility of expand/collapse control. The<button>usestabindex="-1", removing it from the tab order—ensure keyboard users can still trigger expand/collapse (e.g., via arrow-key navigation on the treeitem or another focusable control) for WCAG 2.1.1 compliance.src/components/autocomplete/precompile-templates/autocomplete.handlebars (2)
3-9: LGTM: Correct listbox ARIA implementation.The suggestions list correctly implements the ARIA listbox pattern with:
role="listbox"on the containerrole="option"with unique IDs on each itemaria-selectedreflecting the active state- Semantic alignment between
class="active"andaria-selected="true"
11-20: Live region updates confirmed: TheupdateLiveRegionmethod insrc/components/autocomplete/autocomplete.jscorrectly sets and clears the[role="status"]element’stextContentwith suggestion counts and navigation instructions.src/components/searchresults/precompile-templates/searchresults.handlebars (3)
3-4: LGTM: Proper landmark and live region.The search results region is correctly marked with
role="region"andaria-label, creating an accessible landmark. The live region setup aligns with theupdateLiveRegionmethod insearchresults.jsthat was reviewed earlier.
9-11: Explicit list roles for robustness.The
role="list"on<ol>androle="listitem"on<li>are technically redundant (semantic HTML provides these implicitly), but they ensure list semantics are preserved even if CSS removes list styling (e.g.,list-style: none). This is a defensive accessibility practice.
12-24: LGTM: Semantic search result structure.Each search result is properly structured with:
- Semantic heading (
<h3>) for the title- Preserved analytics tracking (
data-analytics-click)- Appropriate content organization within the list item
- Conditional category display only when multiple categories exist
The transition from
<div>to<li>elements maintains all functionality while improving semantic markup and accessibility.src/components/aianswersresult/precompile-templates/aianswersresult.handlebars (4)
5-18: LGTM: Accessible SVG icon.The AI icon SVG is properly made accessible with
role='img'and a descriptivearia-label, ensuring screen readers announce it meaningfully.
49-66: LGTM: Accessible loading indicator.The loading spinner SVG is properly configured with
role='img'and a descriptivearia-label, informing screen reader users that content is loading.
70-91: LGTM: Accessible copy button.The answer container has an
idfor ARIA relationships, and the copy button is properly labeled witharia-label. The icon is correctly markedaria-hidden='true'.
92-143: LGTM: Accessible toggle buttons with state.The thumbs up/down buttons correctly implement toggle button semantics with:
aria-labeldescribing the actionaria-pressedreflecting the current state- Icons marked
aria-hidden='true'The state management properly reflects whether positive or negative sentiment is selected.
src/components/autocomplete/autocomplete.js (4)
31-31: LGTM!The initialization of
lastAnnouncedCountis appropriate for tracking the announcement state.
140-140: LGTM!Resetting
lastAnnouncedCountensures the next autocomplete appearance will properly announce the suggestion count.
175-176: LGTM!The placement of
updateLiveRegionafter the HTML is rendered ensures the live region element exists in the DOM before attempting to update it.
231-254: Excellent accessibility implementation!The
updateLiveRegionmethod properly:
- Guards against missing live region element
- Handles null/undefined for both
suggestionsandcustomFields- Prevents redundant announcements by tracking
lastAnnouncedCount- Provides clear, actionable screen reader guidance with proper pluralization
src/components/hierarchicalfacets/hierarchicalfacets.scss (3)
72-76: Verify focus outline color contrast.Ensure the focus outline color
#0066ccmeets WCAG 2.2 AA requirements (3:1 contrast ratio) against all background colors where this component appears, including default and theme variations.Consider generating a script to verify the contrast ratio:
#!/usr/bin/env python3 # Verify WCAG contrast ratio for focus outline color def relative_luminance(rgb): """Calculate relative luminance of RGB color""" r, g, b = [x / 255.0 for x in rgb] r = r / 12.92 if r <= 0.03928 else ((r + 0.055) / 1.055) ** 2.4 g = g / 12.92 if g <= 0.03928 else ((g + 0.055) / 1.055) ** 2.4 b = b / 12.92 if b <= 0.03928 else ((b + 0.055) / 1.055) ** 2.4 return 0.2126 * r + 0.7152 * g + 0.0722 * b def contrast_ratio(rgb1, rgb2): """Calculate contrast ratio between two colors""" l1 = relative_luminance(rgb1) l2 = relative_luminance(rgb2) lighter = max(l1, l2) darker = min(l1, l2) return (lighter + 0.05) / (darker + 0.05) # Focus outline color #0066cc focus_color = (0, 102, 204) # Common background colors to test against backgrounds = { 'white': (255, 255, 255), 'light-gray': (220, 220, 220), 'medium-gray': (128, 128, 128), } print("WCAG 2.2 AA requires 3:1 contrast for focus indicators\n") for name, bg_color in backgrounds.items(): ratio = contrast_ratio(focus_color, bg_color) status = "✓ PASS" if ratio >= 3.0 else "✗ FAIL" print(f"{status} {name:15s} - Contrast ratio: {ratio:.2f}:1")
78-82: LGTM!The rotation transform and transition provide smooth, accessible visual feedback for the expansion state.
102-104: LGTM!The rotation reset to 0 degrees provides clear visual feedback for the collapsed state.
src/components/filters/precompile-templates/filters_range.handlebars (3)
1-2: LGTM!The semantic grouping with
role="group"and conditionalaria-labelledbyprovides proper accessibility structure for the range filter.
3-7: LGTM!Excellent use of:
- Screen-reader-only labels for explicit input identification
aria-describedbylinking both inputs to contextual helparia-hidden="true"on the visual separator to prevent screen reader confusion
9-9: LGTM!The screen-reader-only help text with matching ID properly supports the
aria-describedbyassociations, providing contextual guidance to assistive technology users.
src/components/aianswersresult/precompile-templates/aianswersresult.handlebars
Show resolved
Hide resolved
src/components/filters/precompile-templates/filters_range.handlebars
Outdated
Show resolved
Hide resolved
src/components/filters/precompile-templates/filters_tags.handlebars
Outdated
Show resolved
Hide resolved
src/components/pagination/precompile-templates/pagination.handlebars
Outdated
Show resolved
Hide resolved
src/components/rangefacets/precompile-templates/sliderrangefacets.handlebars
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/pagination/precompile-templates/pagination.handlebars (1)
6-6: Usearia-current="page"for better semantic clarity.The current implementation uses
aria-current="true", which is technically valid. However,aria-current="page"is more semantically appropriate for pagination contexts and provides clearer meaning to assistive technologies about the type of current item.Apply this diff:
- <button data-page="{{this}}" aria-label="Go to results page {{this}}" {{#equals ../currentPage this}}data-active="true" aria-current="true"{{/equals}}> + <button data-page="{{this}}" aria-label="Go to results page {{this}}" {{#equals ../currentPage this}}data-active="true" aria-current="page"{{/equals}}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/components/aianswersresult/precompile-templates/aianswersresult.handlebars(9 hunks)src/components/filters/precompile-templates/filters_range.handlebars(1 hunks)src/components/filters/precompile-templates/filters_tags.handlebars(1 hunks)src/components/pagination/precompile-templates/pagination.handlebars(2 hunks)src/components/rangefacets/precompile-templates/sliderrangefacets.handlebars(1 hunks)src/components/searchresults/searchresults.js(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/aianswersresult/precompile-templates/aianswersresult.handlebars
- src/components/filters/precompile-templates/filters_tags.handlebars
- src/components/rangefacets/precompile-templates/sliderrangefacets.handlebars
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: haoAddsearch
PR: AddSearch/search-ui#155
File: src/components/pagination/precompile-templates/pagination.handlebars:1-1
Timestamp: 2025-10-16T13:30:52.737Z
Learning: In the AddSearch Search UI components, prefer using `<div>` with explicit ARIA roles (e.g., `<div role="navigation">`) instead of semantic HTML elements (e.g., `<nav>`) when there's a concern about customer CSS affecting component styling. This maintains accessibility while avoiding potential conflicts with customer stylesheets.
📚 Learning: 2025-10-16T13:30:52.737Z
Learnt from: haoAddsearch
PR: AddSearch/search-ui#155
File: src/components/pagination/precompile-templates/pagination.handlebars:1-1
Timestamp: 2025-10-16T13:30:52.737Z
Learning: In the AddSearch Search UI components, prefer using `<div>` with explicit ARIA roles (e.g., `<div role="navigation">`) instead of semantic HTML elements (e.g., `<nav>`) when there's a concern about customer CSS affecting component styling. This maintains accessibility while avoiding potential conflicts with customer stylesheets.
Applied to files:
src/components/pagination/precompile-templates/pagination.handlebars
🧬 Code graph analysis (1)
src/components/searchresults/searchresults.js (2)
src/actions/search.js (1)
hits(99-99)src/util/history.js (1)
keyword(237-237)
🔇 Additional comments (7)
src/components/pagination/precompile-templates/pagination.handlebars (1)
1-1: LGTM! Proper use of ARIA role per team convention.The use of
<div role="navigation">witharia-label="Search results pagination"correctly provides navigation semantics for assistive technologies while avoiding potential CSS conflicts with customer stylesheets.Based on learnings.
src/components/searchresults/searchresults.js (3)
18-18: LGTM!Initializing
lastAnnouncedResultsfor deduplication tracking is appropriate and follows the pattern used in autocomplete.js.
42-71: Well-implemented live region updates with proper deduplication.The method correctly addresses the previous concern about undefined keywords (line 50 uses
data.keyword || ''). The logic covers all scenarios appropriately:
- No results with a keyword → clear announcement
- Results found → count with optional keyword context
- Initial/empty state → silent
The composite key deduplication prevents announcement spam, and the early returns optimize performance.
106-107: LGTM!Correct placement after HTML injection ensures the live region element exists before updating it.
src/components/filters/precompile-templates/filters_range.handlebars (3)
1-2: LGTM: Proper ARIA grouping and labeling.The
role="group"with conditionalaria-labelledbycorrectly associates the range inputs with their label. Using{{field}}in the ID ensures uniqueness when multiple range filters are present.
3-7: LGTM: Excellent accessibility implementation.The screen-reader-only labels ("Minimum value" and "Maximum value") provide clear context, while
aria-describedbyassociates the help text with both inputs. Hiding the visual separator from screen readers prevents unnecessary announcements.
13-13: LGTM: Clear help text for assistive technology.The help text is properly associated with both inputs via
aria-describedbyand uses the screen-reader-only class to remain hidden from sighted users while providing valuable context to screen reader users.
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/searchresults/searchresults.js (1)
100-109: Bug: live region not updated when HTML is unchanged (early return).
If totalHits/keyword change but template HTML stays identical, announcement is skipped. Call updateLiveRegion even when DOM doesn’t re-render.Apply this diff:
- if (this.renderedHtml === html) { - return; - } - const container = document.getElementById(this.conf.containerId); - container.innerHTML = html; - this.renderedHtml = html; - - // Update live region for screen reader announcements - this.updateLiveRegion(container, data); + const container = document.getElementById(this.conf.containerId); + if (this.renderedHtml === html) { + // Still announce changes even if markup didn't change + this.updateLiveRegion(container, data); + return; + } + container.innerHTML = html; + this.renderedHtml = html; + // Update live region for screen reader announcements + this.updateLiveRegion(container, data);
♻️ Duplicate comments (1)
src/components/searchresults/searchresults.js (1)
50-50: Resolved: guard undefined keyword.
Using empty-string default prevents “undefined” announcements and stabilizes the dedupe key.
🧹 Nitpick comments (2)
src/components/searchresults/searchresults.js (2)
42-47: Target the intended live region explicitly.
querySelector('[role="status"]') may pick the wrong node if templates include multiple status regions. Prefer a dedicated selector (e.g., [data-live-region="results"]) and fall back to role.Example:
- const liveRegion = container.querySelector('[role="status"]'); + const liveRegion = + container.querySelector('[data-live-region="results"]') || + container.querySelector('[role="status"]');Please verify the container renders only one announcing live region to avoid double announcements.
62-71: Optional: externalize announcement strings for i18n.
Hard-coded English (“No search results…”, “result(s)”) hinders localization/plurals. Consider conf-provided strings or a translation utility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/searchfield/searchfield.js(3 hunks)src/components/searchresults/searchresults.js(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/searchfield/searchfield.js
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/searchresults/searchresults.js (2)
src/actions/search.js (1)
hits(99-99)src/reducers/keyword.js (1)
keyword(11-29)
🔇 Additional comments (1)
src/components/searchresults/searchresults.js (1)
18-18: Good: track last announcement to prevent duplicate reads.
Helps avoid noisy screen reader updates.
| @@ -1,16 +1,22 @@ | |||
| <li> | |||
| <li role="treeitem" aria-level="{{level}}" aria-expanded="{{#unless (validateOpenState value)}}true{{else}}false{{/unless}}" {{#if children}}aria-owns="children-{{value}}"{{/if}}> | |||
There was a problem hiding this comment.
The implementation uses role="tree" but mixes it with checkboxes, which violates ARIA patterns. According to https://www.w3.org/WAI/ARIA/apg/patterns/treeview/, trees are for hierarchical navigation, not selection.
Option A: Use role="group" instead of "tree"
Option B: If tree navigation is desired, remove checkboxes and use aria-selected
| <div class="adds-facet-container"> | ||
| <span class="addsearch-facet-group-expansion-arrow {{#unless children}}adds-visibility-hidden{{/unless}}"></span> | ||
| {{#if children}} | ||
| <button type="button" class="addsearch-facet-group-expansion-arrow" aria-label="{{#unless (validateOpenState value)}}Collapse{{else}}Expand{{/unless}} {{displayValue}}" aria-controls="children-{{value}}" tabindex="-1"> |
There was a problem hiding this comment.
Issue: tabindex="-1" removes the button from keyboard navigation, making it inaccessible to keyboard users.
Problem:
- Keyboard-only users cannot expand/collapse hierarchical facets
- Violates WCAG 2.1.1 (Keyboard) - Level A
| const container = e.target.getAttribute('data-container'); | ||
| const confFields = e.target.getAttribute('data-conf-fields') | ||
| ? e.target.getAttribute('data-conf-fields').split(',') | ||
| const targetButton = e.currentTarget; |
There was a problem hiding this comment.
Issue: While the change from e.target to e.currentTarget is correct for button clicks, the handler is directly attached to buttons. If users press Space/Enter on the button, this should work, but there's no verification in the code review of how event listeners are attached.
Recommendation: Verify in activefilters.js that event listeners are attached like:
button.addEventListener('click', this.handleFilterClick);
And ensure keyboard events (Space/Enter) are also handled, though native buttons should handle this automatically.
2 participants
Summary by CodeRabbit
New Features
Refactor
Style
Documentation