-
Notifications
You must be signed in to change notification settings - Fork 12.9k
fix: User Autocomplete's avatar being misaligned #37486
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 61c1593 The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Autocomplete as UserAutoCompleteMultiple
participant Chip as UserAvatarChip
participant Store as SelectedCache
participant Handler as onRemove
User->>Autocomplete: select option
Autocomplete->>Store: add selected (username, name, federated)
Autocomplete->>Chip: render(username, name, federated, onClick=onRemove)
alt federated
Chip->>Chip: show globe icon + name/username
else local
Chip->>Chip: show UserAvatar + name/username
end
User->>Chip: click remove
Chip->>Handler: invoke onRemove(username)
Handler->>Autocomplete: remove selected item
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (3)
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #37486 +/- ##
===========================================
- Coverage 68.96% 68.93% -0.04%
===========================================
Files 3357 3358 +1
Lines 114251 114240 -11
Branches 20536 20573 +37
===========================================
- Hits 78795 78751 -44
- Misses 33369 33399 +30
- Partials 2087 2090 +3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/meteor/client/components/UserAutoCompleteMultiple/UserAvatarChip.tsx (1)
11-20: Clean implementation with good fallback logic.The component correctly handles both federated and non-federated users, and the
name ?? usernamefallback ensures there's always displayable content.Minor: Line 15's
margin='none'appears redundant given thatmis={4}already sets inline-start margin. Consider removing it for clarity:-<Box is='span' margin='none' mis={4} verticalAlign='middle'> +<Box is='span' mis={4} verticalAlign='middle'>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
apps/meteor/client/components/UserAutoCompleteMultiple/__snapshots__/UserAvatarChip.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
.changeset/neat-rats-grab.md(1 hunks)apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx(2 hunks)apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultipleFederated.tsx(2 hunks)apps/meteor/client/components/UserAutoCompleteMultiple/UserAvatarChip.spec.tsx(1 hunks)apps/meteor/client/components/UserAutoCompleteMultiple/UserAvatarChip.stories.tsx(1 hunks)apps/meteor/client/components/UserAutoCompleteMultiple/UserAvatarChip.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/client/components/UserAutoCompleteMultiple/UserAvatarChip.spec.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/client/components/UserAutoCompleteMultiple/UserAvatarChip.spec.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All Playwright test files must be located under apps/meteor/tests/e2e/ and use the .spec.ts extension (e.g., login.spec.ts)
Applied to files:
apps/meteor/client/components/UserAutoCompleteMultiple/UserAvatarChip.spec.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (test, page, expect) consistently
Applied to files:
apps/meteor/client/components/UserAutoCompleteMultiple/UserAvatarChip.spec.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases
Applied to files:
apps/meteor/client/components/UserAutoCompleteMultiple/UserAvatarChip.spec.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Write concise, technical TypeScript/JavaScript with accurate typing
Applied to files:
apps/meteor/client/components/UserAutoCompleteMultiple/UserAvatarChip.spec.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use expect matchers (toEqual, toContain, toBeTruthy, toHaveLength, etc.) instead of assert statements
Applied to files:
apps/meteor/client/components/UserAutoCompleteMultiple/UserAvatarChip.spec.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/client/components/UserAutoCompleteMultiple/UserAvatarChip.spec.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Avoid code comments in the implementation
Applied to files:
apps/meteor/client/components/UserAutoCompleteMultiple/UserAvatarChip.spec.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Follow DRY by extracting reusable logic into helper functions or page objects
Applied to files:
apps/meteor/client/components/UserAutoCompleteMultiple/UserAvatarChip.spec.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (8)
apps/meteor/client/components/UserAutoCompleteMultiple/UserAvatarChip.spec.tsx (2)
1-21: Excellent test coverage with accessibility checks.The combination of snapshot tests and accessibility testing for all Storybook stories provides comprehensive coverage for the component's visual output and accessibility compliance.
23-28: No issues found — the test correctly assumes Chip renders as button.Fuselage's Chip component renders as an interactive element (button) when you provide an onClick prop. Since UserAvatarChip spreads props to Chip (
{...props}), theonClickhandler reaches Chip and causes it to render with button semantics. The test correctly usesscreen.getByRole('button')and will work as written.apps/meteor/client/components/UserAutoCompleteMultiple/UserAvatarChip.tsx (1)
1-9: Well-structured type definition.The component props correctly extend
ComponentProps<typeof Chip>, ensuring all Chip props can be forwarded while adding the specific props needed for user avatar rendering.apps/meteor/client/components/UserAutoCompleteMultiple/UserAvatarChip.stories.tsx (1)
1-43: Comprehensive Storybook coverage.The stories effectively demonstrate all key variations of the component: standard display, federated users, missing names, and non-interactive chips. This provides excellent documentation and visual testing coverage.
.changeset/neat-rats-grab.md (1)
1-5: LGTM!The changeset correctly documents this as a patch-level bugfix with an appropriate description.
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx (2)
1-1: Clean import updates.The transition from inline Chip usage to the dedicated UserAvatarChip component improves code organization and reusability.
Also applies to: 9-9
38-40: Correct integration of UserAvatarChip.The renderSelected callback properly maps the selected user data to UserAvatarChip props, maintaining the removal functionality through the onClick handler.
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultipleFederated.tsx (1)
2-2: Clean import updates.Successfully migrated from inline Chip to the dedicated UserAvatarChip component.
Also applies to: 10-10
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultipleFederated.tsx
Show resolved
Hide resolved
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx
Show resolved
Hide resolved
a0cd65a to
d8c521c
Compare
Proposed changes (including videos or screenshots)
This PR fixes an issue causing the
UserAutoCompleteMultiple's avatar options to become misaligned.Before

After

Issue(s)
FB-26
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Bug Fixes
Tests