refactor(omnichannel): replace SelectLegacy with Select in PrioritiesSelect#38696
Open
NAME-ASHWANIYADAV wants to merge 2 commits intoRocketChat:developfrom
Open
refactor(omnichannel): replace SelectLegacy with Select in PrioritiesSelect#38696NAME-ASHWANIYADAV wants to merge 2 commits intoRocketChat:developfrom
NAME-ASHWANIYADAV wants to merge 2 commits intoRocketChat:developfrom
Conversation
…Select Replaced deprecated SelectLegacy with modern Select component from @rocket.chat/fuselage. Removed custom rendering logic (PriorityIcon) to align with standard Select usage and SlaPoliciesSelect pattern. Added aria-labelledby for better accessibility.
Contributor
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
Contributor
WalkthroughThe PrioritiesSelect component was refactored to use the modern Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. No actionable comments were generated in the recent review. 🎉 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (1)📓 Common learnings⏰ 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 (4)
✏️ Tip: You can disable this entire section by setting 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/meteor/client/views/omnichannel/additionalForms/PrioritiesSelect.tsx`:
- Around line 21-27: The formattedOptions useMemo currently spreads
options?.map(...) which can be undefined and cause a runtime TypeError; update
the mapping to use a safe fallback such as (options ?? []) or
Array.isArray(options) before mapping so the spread always receives an array,
e.g. change the options?.map(...) reference inside the useMemo for
formattedOptions to (options ?? []).map(...) (keeping the same mapping logic for
dirty/name/i18n/_id and preserving SelectOption typing).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/meteor/client/views/omnichannel/additionalForms/PrioritiesSelect.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/views/omnichannel/additionalForms/PrioritiesSelect.tsx
🧬 Code graph analysis (1)
apps/meteor/client/views/omnichannel/additionalForms/PrioritiesSelect.tsx (1)
packages/ui-contexts/src/index.ts (1)
TranslationKey(109-109)
🪛 Biome (2.3.14)
apps/meteor/client/views/omnichannel/additionalForms/PrioritiesSelect.tsx
[error] 24-24: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
⏰ 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: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
🔇 Additional comments (3)
apps/meteor/client/views/omnichannel/additionalForms/PrioritiesSelect.tsx (3)
1-6: Clean import modernization.Imports are well-aligned with the refactor goal — legacy components removed, modern
SelectanduseIdbrought in.
35-42: Good accessibility wiring witharia-labelledby.The
useId-generatedfieldIdproperly connects theFieldLabelandSelect. The JSX structure is clean and matches the pattern used in similar components likeSlaPoliciesSelect.
29-33: Hook ordering is correct — all hooks called before the early return.
useId(and all other hooks) execute unconditionally before the license-gatedreturn null, satisfying the Rules of Hooks.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
apps/meteor/client/views/omnichannel/additionalForms/PrioritiesSelect.tsx
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #38696 +/- ##
===========================================
- Coverage 70.78% 70.46% -0.33%
===========================================
Files 3159 3175 +16
Lines 109364 111068 +1704
Branches 19671 20040 +369
===========================================
+ Hits 77415 78259 +844
- Misses 29920 30763 +843
- Partials 2029 2046 +17
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Use safe spread for options mapping to prevent runtime errors when options is undefined, addressing CodeRabbit review feedback.
NAME-ASHWANIYADAV
force-pushed
the
refactor/priorities-select-legacy
branch
from
a2e5d27 to
5e58fa7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed Changes
Replaced the deprecated
SelectLegacycomponent with the modernSelectcomponent from@rocket.chat/fuselageinPrioritiesSelect.tsx.SelectLegacytoSelect.renderOptions,renderSelected,renderItem) and thePriorityIconlogic, aligning with standardSelectusage and the pattern used inSlaPoliciesSelect.tsx.aria-labelledbyto theSelectcomponent.Issue(s)
Closes #38695
Steps to Test or Reproduce
Selectcomponent.Further Comments
This change aligns
PrioritiesSelectwith the modern Fuselage component library usage, removing reliance on legacy components and simplifying the maintenance of the Omnichannel codebase.Summary by CodeRabbit