-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat(18222): add transformSearchResults prop to MultiselectChipWithSearch #91
feat(18222): add transformSearchResults prop to MultiselectChipWithSearch #91
Conversation
Warning Rate limit exceeded@albaranau has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 12 seconds before requesting another review. 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. WalkthroughThe Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
♻️ PR Preview 03d7340 has been successfully destroyed since this PR has been closed. 🤖 By surge-preview |
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
Outside diff range comments (3)
packages/ui-kit/src/Select/components/MultiselectChipWithSearch/index.tsx (3)
Line range hint
206-206
: Specify the button type explicitly to avoid unintended form submissions.The button within the component does not specify its type, which defaults to 'submit' and can cause unintended form submissions. Consider setting the type explicitly:
- <button onClick={reset}> + <button type="button" onClick={reset}>
Line range hint
210-210
: Use self-closing tags for elements without children.For cleaner code and to adhere to JSX best practices, use self-closing tags for elements that do not have children:
- <div className={btnStyle.delimiter}></div> + <div className={btnStyle.delimiter} />
Line range hint
231-231
: Replace template literals with string literals when interpolation is not needed.The use of template literals here is unnecessary since there's no dynamic content. Simplify this to a string literal:
- key={`${item.value}${index}`} + key={item.value + index}
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
Files selected for processing (1)
- packages/ui-kit/src/Select/components/MultiselectChipWithSearch/index.tsx (4 hunks)
Additional context used
Biome
packages/ui-kit/src/Select/components/MultiselectChipWithSearch/index.tsx
[error] 210-210: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing. (lint/style/useSelfClosingElements)
Unsafe fix: Use a SelfClosingElement instead
[error] 231-231: Do not use template literals if interpolation and special-character handling are not needed. (lint/style/noUnusedTemplateLiteral)
Unsafe fix: Replace with string literal
[error] 81-81: This hook does not specify all of its dependencies: transformSearchResults (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
This dependency is not specified in the hook dependency list.
[error] 206-206: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)
The default type of a button is submit, which causes the submission of a form when placed inside a
form
element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
Additional comments not posted (1)
packages/ui-kit/src/Select/components/MultiselectChipWithSearch/index.tsx (1)
Line range hint
58-76
: Ensure the newtransformSearchResults
prop is properly documented.The addition of the
transformSearchResults
prop is well implemented and aligns with the PR objectives. However, consider adding more detailed documentation or examples in the JSDoc to help developers understand how to use this new prop effectively.Tools
Biome
[error] 81-81: This hook does not specify all of its dependencies: transformSearchResults (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
This dependency is not specified in the hook dependency list.
const resultItems = useMemo(() => { | ||
const filtered = getFilteredList({ | ||
list: items, | ||
exclude: selectedItems, | ||
search: inputValue, | ||
filter, | ||
}); | ||
if (transformSearchResults) { | ||
transformSearchResults(filtered, inputValue); | ||
} | ||
return filtered; | ||
}, [items, selectedItems, filter, inputValue]); |
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.
Add transformSearchResults
to the dependency array of useMemo
.
The useMemo
hook is missing transformSearchResults
in its dependency array. This could lead to stale closures if transformSearchResults
changes. Here's the suggested fix:
- }, [items, selectedItems, filter, inputValue]);
+ }, [items, selectedItems, filter, inputValue, transformSearchResults]);
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
const resultItems = useMemo(() => { | |
const filtered = getFilteredList({ | |
list: items, | |
exclude: selectedItems, | |
search: inputValue, | |
filter, | |
}); | |
if (transformSearchResults) { | |
transformSearchResults(filtered, inputValue); | |
} | |
return filtered; | |
}, [items, selectedItems, filter, inputValue]); | |
const resultItems = useMemo(() => { | |
const filtered = getFilteredList({ | |
list: items, | |
exclude: selectedItems, | |
search: inputValue, | |
filter, | |
}); | |
if (transformSearchResults) { | |
transformSearchResults(filtered, inputValue); | |
} | |
return filtered; | |
}, [items, selectedItems, filter, inputValue, transformSearchResults]); |
Tools
Biome
[error] 81-81: This hook does not specify all of its dependencies: transformSearchResults (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
This dependency is not specified in the hook dependency list.
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
Outside diff range comments (3)
packages/ui-kit/src/Select/components/MultiselectChipWithSearch/index.tsx (3)
Line range hint
210-210
: JSX elements without children should be self-closing.- <div className={btnStyle.delimiter}></div> + <div className={btnStyle.delimiter} />
Line range hint
231-231
: Avoid unnecessary template literals.- key={`no-data`} + key="no-data"
Line range hint
206-206
: Specify the type attribute for button elements to prevent unexpected form submissions.- <button onClick={reset}> + <button type="button" onClick={reset}>
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (1)
- packages/ui-kit/src/Select/components/MultiselectChipWithSearch/index.tsx (4 hunks)
Additional context used
Biome
packages/ui-kit/src/Select/components/MultiselectChipWithSearch/index.tsx
[error] 210-210: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing. (lint/style/useSelfClosingElements)
Unsafe fix: Use a SelfClosingElement instead
[error] 231-231: Do not use template literals if interpolation and special-character handling are not needed. (lint/style/noUnusedTemplateLiteral)
Unsafe fix: Replace with string literal
[error] 206-206: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)
The default type of a button is submit, which causes the submission of a form when placed inside a
form
element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
Additional comments not posted (1)
packages/ui-kit/src/Select/components/MultiselectChipWithSearch/index.tsx (1)
Line range hint
58-76
: EnsuretransformSearchResults
is used correctly in theuseMemo
hook.Verification successful
The
transformSearchResults
function is used within theuseMemo
hook and is correctly included in the dependency array. Here are the relevant lines from theindex.tsx
file:
- Line 70:
transformSearchResults
is defined as a prop.- Line 95:
transformSearchResults
is used within anif
statement.- Line 96:
transformSearchResults(filtered, inputValue);
is called.- Line 97:
transformSearchResults
is included in the dependency array of theuseMemo
hook.These usages indicate that
transformSearchResults
is correctly implemented and used within theuseMemo
hook.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `transformSearchResults` is used correctly in the `useMemo` hook. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type typescript $'transformSearchResults'Length of output: 80
Script:
#!/bin/bash # Description: Verify that `transformSearchResults` is used correctly in the `useMemo` hook. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ts --type tsx 'transformSearchResults'Length of output: 76
Script:
#!/bin/bash # Description: Verify that `transformSearchResults` is used correctly in the `useMemo` hook. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ts 'transformSearchResults' rg --type tsx 'transformSearchResults'Length of output: 724
packages/ui-kit/src/Select/components/MultiselectChipWithSearch/index.tsx
Outdated
Show resolved
Hide resolved
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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (1)
- packages/ui-kit/src/Select/components/MultiselectChipWithSearch/index.tsx (4 hunks)
Additional comments not posted (6)
packages/ui-kit/src/Select/components/MultiselectChipWithSearch/index.tsx (6)
Line range hint
9-12
: Utility functionsdefaultFilterByTitle
anddefaultItemToString
are well-implemented and clear.Also applies to: 14-16
Line range hint
18-33
: FunctiongetFilteredList
is correctly implemented and efficiently filters items based on the provided criteria.
76-76
: The new proptransformSearchResults
is well-integrated and provides the necessary functionality for custom result transformations.
92-92
: The dependency array foruseMemo
correctly includestransformSearchResults
, ensuring that the memoized value updates when any dependency changes.
Line range hint
121-217
: The rendering logic and event handling in the component are robust, handling various states and user interactions effectively.
217-217
: The conditional rendering ofSelectItem
components is correctly implemented, ensuring an appropriate UI response when no items are available.
Adding prop to pass result transformation function. It's required to pass complex custom sorting to the component.
https://kontur.fibery.io/Tasks/Task/Show-layers-starting-with-key-words-at-the-top-in-MCDA-dropdown-18222
Summary by CodeRabbit
MultiselectChipWithSearch
component to include atransformSearchResults
function, allowing for custom modifications to filtered search results.