-
Notifications
You must be signed in to change notification settings - Fork 429
Fix React DOM nesting and prop warnings #5335
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
base: main
Are you sure you want to change the base?
Conversation
redisinsight/ui/src/pages/browser/components/redisearch-key-list/RediSearchIndexesList.tsx
Show resolved
Hide resolved
6a80c64 to
7a37365
Compare
Shared utility to filter out customColor prop that some parent components pass to icons, preventing React warnings on SVG elements. Uses WeakMap caching to ensure the same input icon always returns the same wrapped component, preventing React reconciliation issues.
Use iconWithoutCustomColor utility to prevent React warnings when customColor prop is passed to SVG elements that don't support it.
Add component prop to Text components to prevent invalid HTML nesting. Text renders as <p> by default which cannot contain block elements.
Use styled span with role=button instead of IconButton to avoid button inside button DOM nesting error in select trigger.
Add key prop to mapped elements to prevent React warning.
Replace props spread with explicit row prop to prevent passing invalid attributes to checkbox element.
7a37365 to
f234c03
Compare
Prevent duplicate refresh requests when keyboard event fires while element still has focus during loading state.
Code Coverage - Frontend unit tests
Test suite run success5486 tests passing in 704 suites. Report generated by 🧪jest coverage report action from cf28d76 |
| /> | ||
| </RiTooltip> | ||
| </div> | ||
| <RiTooltip content="Refresh Indexes"> |
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.
This is the file with an actual change, but the UI stays the same.
| </Text> | ||
| ) : ( | ||
| noItemsMessage | ||
| )} |
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.
Bug: Missing text styling for noItemsMessage when not loading
The noRowsRenderer function was refactored to conditionally wrap content with Text, but now when loading is false, the noItemsMessage is rendered without the Text wrapper that previously provided textAlign="center" and size="m" styling. While the flex container .placeholder provides centering via flexbox, if noItemsMessage is a plain string (like the default 'No keys to display.'), it loses the text size styling. The original code wrapped both the loading text and noItemsMessage in a single Text component with consistent styling.
| tabIndex={loading ? -1 : 0} | ||
| aria-disabled={loading} | ||
| aria-label="refresh indexes list" | ||
| onClick={handleRefresh} |
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.
Bug: Click handler missing loading check unlike keyboard handler
The handleRefresh click handler dispatches fetchRedisearchListAction() without checking the loading state, while the handleRefreshKeyDown keyboard handler properly checks if (loading) return before dispatching. The original IconButton used native disabled={loading} which reliably prevents clicks, but the new implementation relies solely on CSS pointer-events: none to block clicks when disabled. If CSS fails to apply or is overridden, clicks could trigger duplicate requests during loading. The inconsistency between handlers suggests the click handler is missing the same defensive check.
What
Fixes various React console warnings related to DOM nesting and invalid props:
iconWithoutCustomColorutility to filter props that SVG elements don't supportcomponentprop to prevent<p>inside<p>or<div>inside<p>warningsIconButtonwith styled<span role="button">in RediSearchIndexesListTesting
Closes N/A
Pull Request opened by Augment Code with guidance from the PR author
Note
Normalize
Textsemantics, filter unsupported icon props, and replace RediSearch refreshIconButtonwith an accessible span; also add missing keys and adjust table selection props.iconWithoutCustomColorwith WeakMap caching incomponents/base/icons/Icon.tsxto strip unsupportedcustomColor.Button.IconandSideBarItemIcon.Textcomponenttospan/divacross components to fix invalid nesting:RiToast, bottom group badges, notifications, upload warning,VirtualTablecells/empty state, key row name/size, success messages, and no-key-selected placeholder.IconButtonrefresh with styledspan(RefreshAction) + keyboard handlers; add styles inRediSearchIndexesList.styles.ts.keyto mapped buttons inExploreGuides.rowtoTable.RowSelectionButtonin databases list config.Written by Cursor Bugbot for commit cf28d76. This will update automatically on new commits. Configure here.