feat(filters): add contact filter - #4111
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRefines the contact filter by requiring avatars in option data, removing the kind discriminator, and retaining normalized filtering capped at 20 results. Extends the shared Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
BundleMonUnchanged files (18)
Total files change +4B 0% Groups updated (1)
Unchanged groups (2)
Final result: ✅ View report in BundleMon website ➡️ |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/Filters/ContactFilter.tsx (1)
256-282: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAutosuggest defaults to a non-unique
id.
menuIdis already generated per-instance for the wrapper/aria-controls, but the<Autosuggest>itself isn't given anid, so it falls back to the library's static default. Per the react-autosuggest docs, multiple simultaneously-rendered instances must use unique ids to avoid collidingaria-owns/aria-activedescendanttargets.♿ Proposed fix
<Autosuggest<ContactFilterOption, never> alwaysRenderSuggestions focusInputOnSuggestionClick={false} getSuggestionValue={getSuggestionValue} highlightFirstSuggestion + id={menuId} inputProps={{Based on external documentation: "When rendering a single Autosuggest, don't set the id (it will be set to '1', by default). When rendering multiple Autosuggest components on a page, make sure to give them unique ids."
🤖 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 `@src/components/Filters/ContactFilter.tsx` around lines 256 - 282, Pass the existing per-instance menuId as the unique id prop on the Autosuggest component, alongside its current configuration. Do not change the generated menuId or surrounding aria-controls usage; reuse that identifier to prevent duplicate react-autosuggest accessibility targets.
🤖 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.
Nitpick comments:
In `@src/components/Filters/ContactFilter.tsx`:
- Around line 256-282: Pass the existing per-instance menuId as the unique id
prop on the Autosuggest component, alongside its current configuration. Do not
change the generated menuId or surrounding aria-controls usage; reuse that
identifier to prevent duplicate react-autosuggest accessibility targets.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8ab019cb-267a-4c1b-a96a-98e4f2ddb9ea
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
package.jsonsrc/components/Filters/ContactFilter.spec.tsxsrc/components/Filters/ContactFilter.stylsrc/components/Filters/ContactFilter.tsxsrc/components/Filters/Filter.jsxsrc/components/Filters/contactFilterOptions.tssrc/components/Filters/index.jssrc/locales/en.jsonsrc/locales/fr.jsonsrc/locales/ru.jsonsrc/locales/vi.json
0d28b94 to
c4eefdd
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
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 `@src/components/Filters/ContactFilter.styl`:
- Around line 38-46: Add a keyboard-visible focus indicator for the search field
by introducing a :focus-visible rule on .searchInput or a :focus-within rule on
.search, while retaining the existing visual reset for non-focused states.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c7eba414-1a25-4c12-8ab0-11c25c91fb79
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
package.jsonsrc/components/Filters/ContactFilter.spec.tsxsrc/components/Filters/ContactFilter.stylsrc/components/Filters/ContactFilter.tsxsrc/components/Filters/Filter.jsxsrc/components/Filters/contactFilterOptions.tssrc/components/Filters/index.jssrc/locales/en.jsonsrc/locales/fr.jsonsrc/locales/ru.jsonsrc/locales/vi.json
🚧 Files skipped from review as they are similar to previous changes (8)
- src/components/Filters/index.js
- src/locales/en.json
- src/locales/ru.json
- src/locales/vi.json
- src/components/Filters/Filter.jsx
- src/components/Filters/ContactFilter.spec.tsx
- src/components/Filters/contactFilterOptions.ts
- src/components/Filters/ContactFilter.tsx
|
@shepilov why using a new intermediate type ContactFilterOption? As input, you have We have helper and existing components that can help for this kind of UI like |
@zatteo because I don't know what model we will have there, I haven't looked yet what we have inside the sharings. But it's supposed to be not all the contacts from the addressbook, but a set of members from the existing shares |
c4eefdd to
5f5c175
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
If you are more in a "build UI components" mode, let's go like this for now. But if at the end we get io.cozy.contacts and io.cozy.contacts.group OR sharing members, we may more components to mutualize/use from cozy-ui/cozy-sharing. For example there is already an "autocomplete on contact or group" in the sharing modal. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
yes, at first I was going to use "autocomplete on contact or group" on a sharing modal, but it has different behavior then the existing one; we too many customizations in it, like the source of the option, and how the selected results should be displayed, multiselect ant etc, we can discuss how we can mutualize it, but for me it looked to many differences |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Filters/ContactFilter.tsx (1)
155-186: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReset the menu state when the filter becomes disabled.
The conditional
onClickblocks new opens, but an already-open menu leavesisOpenset totrue. Togglingdisabledwhile open can leavearia-expandedand the icon inconsistent with the hidden menu, then reopen the menu with the stale query when re-enabled. Close/reset the state on disable and derive all open-state UI fromisOpen && !disabled.🤖 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 `@src/components/Filters/ContactFilter.tsx` around lines 155 - 186, Update the ContactFilter component’s disabled-state handling to close/reset the menu when disabled becomes true, clearing any stale query as appropriate. Derive aria-expanded, the trigger icon state, and menu visibility from isOpen && !disabled so all open-state UI remains synchronized and reopening starts cleanly.
🤖 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.
Outside diff comments:
In `@src/components/Filters/ContactFilter.tsx`:
- Around line 155-186: Update the ContactFilter component’s disabled-state
handling to close/reset the menu when disabled becomes true, clearing any stale
query as appropriate. Derive aria-expanded, the trigger icon state, and menu
visibility from isOpen && !disabled so all open-state UI remains synchronized
and reopening starts cleanly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f334d767-287a-4dc2-abde-4d1c4e2bc3eb
📒 Files selected for processing (6)
src/components/Filters/ContactFilter.spec.tsxsrc/components/Filters/ContactFilter.stylsrc/components/Filters/ContactFilter.tsxsrc/components/Filters/ContactFilterSuggestion.tsxsrc/components/Filters/ContactFilterSuggestionsContainer.tsxsrc/components/Filters/contactFilterOptions.ts
💤 Files with no reviewable changes (1)
- src/components/Filters/ContactFilter.styl
JF-Cozy
left a comment
JF-Cozy
left a comment
There was a problem hiding this comment.
approve to unlock but comments should be considered :) welcome to the frontend world, and well done so far 👍
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
f35cbd3 to
589aa8b
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
589aa8b to
addef43
Compare
There was a problem hiding this comment.
Our agent can fix these. Install it.
Gates Passed
Quality Gate Profile: The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
4 participants
Summary
Summary by CodeRabbit
Filtercomponent to support optionalInputPropsand configurable selectable behavior.