feat(filter,status-indicator): new filter component, status-indicator #329#396
feat(filter,status-indicator): new filter component, status-indicator #329#396mart-sessman wants to merge 21 commits into
Conversation
…ready # Conflicts: # .claude/hooks/post-edit-test.sh # .claude/skills/contributing/SKILL.md # .claude/skills/contributing/references/a11y-review.md # .claude/skills/contributing/references/best-practices.md # .claude/skills/contributing/references/new-component.md # .claude/skills/contributing/references/refactoring.md # .claude/skills/contributing/references/stories.md # .claude/skills/contributing/references/testing.md # CLAUDE.md # skills/tedi-angular/references/components.md # skills/tedi-angular/references/forms.md # tedi/components/form/index.ts
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a new Filter component family (FilterComponent, FilterGroupComponent, prepend/content directives, styles, tests, stories, and barrel exports), a StatusIndicator component and StatusBadge integration, Storybook reactive-form state standardization, dropdown accessibility tweaks (aria/tabindex), docs/best-practices updates for icon color behavior, and small CSS adjustments. ChangesFilter Component Family
Status Indicator & Badge
Storybook Reactive-form State Standardization & DatePicker Story
Dropdown Accessibility & Small Tweaks
Docs & Reference Updates
Sequence Diagram(s)sequenceDiagram
autonumber
rect rgba(240,240,255,0.5)
participant User
end
rect rgba(200,255,200,0.5)
participant FilterComponent
participant DropdownPanel
end
rect rgba(255,240,200,0.5)
participant FormControl
participant FilterGroup
end
User->>FilterComponent: click trigger / keyboard open
FilterComponent->>DropdownPanel: open() / render options (search/select-all)
User->>DropdownPanel: select option / toggle checkbox
DropdownPanel->>FilterComponent: notify selection/toggle
FilterComponent->>FormControl: onChange(newValue)
FormControl-->>FilterComponent: value updated
FilterComponent->>FilterGroup: update selection state (if grouped)
FilterGroup-->>FilterComponent: reflect selected styling/aria
FilterComponent->>User: close dropdown / focus trigger
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
.claude/skills/contributing/references/best-practices.md (1)
275-289: Consider making the story snippet fully copy-pasteable.The example uses
<tedi-alert>andtedi-text; adding explicitAlertComponentandTextComponentin the shownmoduleMetadata.importswould reduce ambiguity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/contributing/references/best-practices.md around lines 275 - 289, Update the story example to be fully copy-pasteable by adding the missing AlertComponent and TextComponent to the story's moduleMetadata imports; specifically ensure the moduleMetadata block shown alongside the template imports AlertComponent and TextComponent so the <tedi-alert> and <tedi-text> usages resolve when consumers paste the snippet, and keep the existing template (containing <tedi-my-control>, <tedi-alert>, and the <pre tedi-text ...>{{ ... | json }}</pre>) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/tedi-angular/references/components.md`:
- Around line 368-369: Update the docs to clarify that [tediFilterPrepend] is
toggle-only: it is hidden when the filter component’s isSelected() returns true
and is not always replaced by a check icon in dropdown modes—note that the check
icon is rendered only in the non-dropdown branch of
tedi/components/form/filter/filter.component.html—so change the sentence about
replacement to state that the prepend content is removed when selected and only
replaced by a check icon in the non-dropdown (non-dropdown single/multi-select)
rendering path; keep the [tediFilterContent] description as-is.
In `@tedi/components/form/filter/filter-group.component.scss`:
- Around line 4-31: The stylesheet for filter-group currently reaches into
tedi-filter's internals by styling .tedi-filter__button; change this so
filter-group only targets host-level modifiers on the tedi-filter component
instead: add and use classes like .tedi-filter--grouped,
.tedi-filter--grouped-first, .tedi-filter--grouped-last and
.tedi-filter--grouped-only on the tedi-filter host (set by the filter-group
component) and update the selectors in filter-group.component.scss to use
.tedi-filter--grouped (and the -first/-last/-only variants) instead of
.tedi-filter__button so styling no longer depends on tedi-filter's internal DOM
structure.
In `@tedi/components/form/filter/filter.component.html`:
- Around line 11-12: The ArrowUp handler should open the dropdown and focus the
last item instead of calling the same callback as ArrowDown; change the
(keydown.arrowUp) binding to call focusDropdownContent(false) (or an equivalent
parameter/flag) so DropdownTriggerDirective can open on the last option while
(keydown.arrowDown) continues to call focusDropdownContent(true).
In `@tedi/components/form/filter/filter.component.ts`:
- Around line 420-434: handleTabKey is failing when focus lands on the
multiselect "Select all" row because getTabStops omits elements with
role="checkbox" (the select-all) so tabStops.indexOf returns -1 and wrapping
logic misbehaves; update getTabStops() to include the select-all checkbox
element (elements with role="checkbox" and tabindex="0" inside the multiselect)
so that handleTabKey() correctly finds the currentIndex and Shift+Tab/Tab wrap
to the proper elements; reference getTabStops() and handleTabKey() when making
the change and ensure the same inclusion fixes the behavior described around the
other affected region (lines 457-465).
- Around line 34-38: The code currently uses raw FilterOption.value for DOM ids
which can produce invalid/duplicate IDs; update the filter component to generate
a stable internal id per option instead of concatenating the raw value: either
add an optional id field to the FilterOption interface or, preferably, compute a
deterministic internal id (e.g., prefix + index or prefix + short hash of value)
when mapping options in the filter component (e.g., in the code that renders
options and in the logic that sets aria-activedescendant / id on elements around
FilterOption usage); replace all uses of FilterOption.value for id/aria
references (including the spots mentioned around lines 266-268) to use this
generated id so IDs are valid and unique even when values contain spaces or
duplicates.
In `@tedi/components/tags/index.ts`:
- Line 3: Replace the direct component-file export in the tags barrel with the
component-level barrel export: change the statement that exports from
"./status-indicator/status-indicator.component" to export from
"./status-indicator" so the tags index re-exports via the status-indicator index
and preserves the documented export chain (public-api → tedi → category index →
component index).
In `@tedi/components/tags/status-indicator/status-indicator.component.ts`:
- Around line 19-30: The host currently sets aria-hidden="true" making
meaningful dots inaccessible; add an `@Input`() (e.g., ariaLabel or label) on
StatusIndicatorComponent and bind the host aria-hidden and aria-label to it so
the host is hidden only when no accessible name is provided (e.g., bind
"aria-hidden": "!ariaLabel" and "attr.aria-label": "ariaLabel || null"), update
the host metadata in status-indicator.component (the same block with type(),
size(), hasBorder(), position()) and ensure the new input is documented/optional
so the indicator remains decorative by default but exposes an accessible name
when provided.
---
Nitpick comments:
In @.claude/skills/contributing/references/best-practices.md:
- Around line 275-289: Update the story example to be fully copy-pasteable by
adding the missing AlertComponent and TextComponent to the story's
moduleMetadata imports; specifically ensure the moduleMetadata block shown
alongside the template imports AlertComponent and TextComponent so the
<tedi-alert> and <tedi-text> usages resolve when consumers paste the snippet,
and keep the existing template (containing <tedi-my-control>, <tedi-alert>, and
the <pre tedi-text ...>{{ ... | json }}</pre>) unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 17b49af5-2731-48c2-a050-99534491e073
📒 Files selected for processing (26)
.claude/skills/contributing/references/best-practices.mdCLAUDE.mdskills/tedi-angular/references/components.mdtedi/components/form/date-picker/date-picker.stories.tstedi/components/form/filter/filter-content.directive.tstedi/components/form/filter/filter-group.component.scsstedi/components/form/filter/filter-group.component.tstedi/components/form/filter/filter.component.htmltedi/components/form/filter/filter.component.scsstedi/components/form/filter/filter.component.spec.tstedi/components/form/filter/filter.component.tstedi/components/form/filter/filter.stories.tstedi/components/form/filter/index.tstedi/components/form/index.tstedi/components/overlay/dropdown/dropdown-item-value/dropdown-item-value.component.htmltedi/components/overlay/dropdown/dropdown-trigger/dropdown-trigger.directive.tstedi/components/tags/index.tstedi/components/tags/status-badge/status-badge.component.htmltedi/components/tags/status-badge/status-badge.component.scsstedi/components/tags/status-badge/status-badge.component.spec.tstedi/components/tags/status-badge/status-badge.component.tstedi/components/tags/status-indicator/index.tstedi/components/tags/status-indicator/status-indicator.component.scsstedi/components/tags/status-indicator/status-indicator.component.spec.tstedi/components/tags/status-indicator/status-indicator.component.tstedi/components/tags/status-indicator/status-indicator.stories.ts
💤 Files with no reviewable changes (1)
- tedi/components/tags/status-badge/status-badge.component.scss
|
@coderabbitai resolve |
✅ Actions performedComments resolved. Approval is disabled; enable |
# Conflicts: # skills/tedi-angular/references/components.md # tedi/components/form/checkbox-card/checkbox-card.component.scss # tedi/components/form/checkbox-card/checkbox-card.component.spec.ts # tedi/components/form/checkbox-card/checkbox-card.component.ts # tedi/components/form/checkbox/checkbox.stories.ts # tedi/components/form/radio-card/radio-card.component.scss # tedi/components/form/radio-card/radio-card.component.spec.ts # tedi/components/form/radio-card/radio-card.component.ts # tedi/components/form/radio/radio.stories.ts
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tedi/components/form/filter/filter.component.html (2)
127-146: Consider applying--selectedmodifier class to multi-select options too.The single-select item block (line 61) sets
[class.tedi-filter-dropdown__item--selected]="isOptionSelected(option.value)", but the multi-select item block omits it. If styles for__item--selectedexist, multi-select options won't get that visual treatment; the checkbox insidetedi-dropdown-item-valuestill renders, but the row-level styling becomes inconsistent. Either drop the class from single-select or add it here for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tedi/components/form/filter/filter.component.html` around lines 127 - 146, The multi-select option block is missing the row-level selected class causing inconsistent visuals; add the same class binding used in the single-select block to the multi-select div by setting [class.tedi-filter-dropdown__item--selected]="isOptionSelected(option.value)" alongside the existing bindings (the div rendered in the for loop that uses filteredOptions(), activeOptionIndex(), getOptionId(), toggleOption(), and isOptionSelected()) so selected rows receive the __item--selected modifier consistently with single-select items.
94-113: Select-all keyboard handling is minimal.
role="checkbox"with only Enter/Space works, but the select-all row is outside thelistboxand doesn't participate in the optionactiveOptionIndexnavigation. Users pressing ArrowDown on the search field / select-all won't move into the listbox. Consider whether focusing the options list from select-all via ArrowDown (or making select-all the first option in the listbox witharia-checked="mixed") would improve the keyboard flow. Not blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tedi/components/form/filter/filter.component.html` around lines 94 - 113, The select-all row uses role="checkbox" and only handles Enter/Space via toggleSelectAll(), but doesn't integrate with listbox navigation; update the element handling in filter.component.html so ArrowDown (and ArrowUp where appropriate) move focus into the options list or treat the select-all as the first listbox option: add keyboard handlers for keydown.arrowdown (and arrowup if needed) to call a new method (e.g., focusFirstOption()) and/or change the container semantics to include the select-all in the listbox flow; update or add methods referenced (toggleSelectAll(), showSelectAll(), allFilteredSelected(), someFilteredSelected(), selectAllLabel()) to support focusing the options list programmatically so Arrow keys behave consistently with the option activeOptionIndex navigation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tedi/components/form/filter/filter.stories.ts`:
- Around line 23-50: The default export for the story (the object that includes
title/component/decorators for FilterComponent) is missing Storybook status
parameters; add a parameters property to that default export with the required
TEDI readiness flags (e.g., partiallyTediReady, existsInTediReady, devComponent)
and appropriate boolean values so the component's status surfaces in Storybook;
update the same default export object where FilterComponent is defined to
include these parameters.
---
Nitpick comments:
In `@tedi/components/form/filter/filter.component.html`:
- Around line 127-146: The multi-select option block is missing the row-level
selected class causing inconsistent visuals; add the same class binding used in
the single-select block to the multi-select div by setting
[class.tedi-filter-dropdown__item--selected]="isOptionSelected(option.value)"
alongside the existing bindings (the div rendered in the for loop that uses
filteredOptions(), activeOptionIndex(), getOptionId(), toggleOption(), and
isOptionSelected()) so selected rows receive the __item--selected modifier
consistently with single-select items.
- Around line 94-113: The select-all row uses role="checkbox" and only handles
Enter/Space via toggleSelectAll(), but doesn't integrate with listbox
navigation; update the element handling in filter.component.html so ArrowDown
(and ArrowUp where appropriate) move focus into the options list or treat the
select-all as the first listbox option: add keyboard handlers for
keydown.arrowdown (and arrowup if needed) to call a new method (e.g.,
focusFirstOption()) and/or change the container semantics to include the
select-all in the listbox flow; update or add methods referenced
(toggleSelectAll(), showSelectAll(), allFilteredSelected(),
someFilteredSelected(), selectAllLabel()) to support focusing the options list
programmatically so Arrow keys behave consistently with the option
activeOptionIndex navigation.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0a5f25d1-d2f4-49b9-8f35-cc6acdcf3da6
📒 Files selected for processing (16)
.claude/skills/contributing/references/best-practices.mdskills/tedi-angular/references/components.mdtedi/components/form/filter/filter-group.component.scsstedi/components/form/filter/filter-group.component.spec.tstedi/components/form/filter/filter-group.component.tstedi/components/form/filter/filter-prepend.directive.tstedi/components/form/filter/filter.component.htmltedi/components/form/filter/filter.component.scsstedi/components/form/filter/filter.component.spec.tstedi/components/form/filter/filter.component.tstedi/components/form/filter/filter.stories.tstedi/components/form/filter/index.tstedi/components/form/radio-card/radio-card.component.scsstedi/components/tags/index.tstedi/components/tags/status-indicator/status-indicator.component.spec.tstedi/components/tags/status-indicator/status-indicator.component.ts
✅ Files skipped from review due to trivial changes (4)
- tedi/components/form/radio-card/radio-card.component.scss
- tedi/components/form/filter/filter-group.component.scss
- tedi/components/form/filter/filter.component.scss
- skills/tedi-angular/references/components.md
🚧 Files skipped from review as they are similar to previous changes (5)
- tedi/components/tags/index.ts
- tedi/components/form/filter/index.ts
- tedi/components/form/filter/filter.component.spec.ts
- tedi/components/form/filter/filter-group.component.ts
- tedi/components/form/filter/filter.component.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tedi/components/form/filter/filter.component.html`:
- Around line 137-155: The multi-select loop rendering options (the `@for` block
using filteredOptions(), activeOptionIndex(), isOptionSelected(),
toggleOption(), getOptionId()) is missing the row-level selected class binding;
add
[class.tedi-filter-dropdown__item--selected]="isOptionSelected(option.value)" to
the containing div (the element with class "tedi-filter-dropdown__item") so
selected multi-select rows receive the same visual styling as single-select
rows.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4e5b193e-df75-4dc7-a537-e523f19abb2e
📒 Files selected for processing (4)
tedi/components/form/filter/filter.component.htmltedi/components/form/filter/filter.component.scsstedi/components/form/filter/filter.component.spec.tstedi/components/form/filter/filter.component.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- tedi/components/form/filter/filter.component.spec.ts
- tedi/components/form/filter/filter.component.ts
Also separated status-indicator from status-badge
Summary by CodeRabbit