Skip to content

Conversation

@pawelangelow
Copy link
Contributor

@pawelangelow pawelangelow commented Dec 17, 2025

What

Fixes various React console warnings related to DOM nesting and invalid props:

  • customColor prop: Added shared iconWithoutCustomColor utility to filter props that SVG elements don't support
  • Text component nesting: Added component prop to prevent <p> inside <p> or <div> inside <p> warnings
  • Button inside button: Replaced IconButton with styled <span role="button"> in RediSearchIndexesList
  • Missing key prop: Added key to mapped elements in ExploreGuides
  • Props spread: Use explicit props instead of spreading in DatabasesList checkbox

Testing

  1. Open the app and navigate through different pages
  2. Open browser console - no DOM nesting or prop warnings should appear
  3. Verify all UI elements look and function the same as before

Closes N/A


Pull Request opened by Augment Code with guidance from the PR author


Note

Normalize Text semantics, filter unsupported icon props, and replace RediSearch refresh IconButton with an accessible span; also add missing keys and adjust table selection props.

  • Icons:
    • Add iconWithoutCustomColor with WeakMap caching in components/base/icons/Icon.tsx to strip unsupported customColor.
    • Use filtered icons in Button.Icon and SideBarItemIcon.
  • Accessibility / DOM semantics:
    • Set Text component to span/div across components to fix invalid nesting: RiToast, bottom group badges, notifications, upload warning, VirtualTable cells/empty state, key row name/size, success messages, and no-key-selected placeholder.
  • RediSearch Indexes List:
    • Replace IconButton refresh with styled span (RefreshAction) + keyboard handlers; add styles in RediSearchIndexesList.styles.ts.
  • Misc:
    • Add key to mapped buttons in ExploreGuides.
    • Pass explicit row to Table.RowSelectionButton in databases list config.

Written by Cursor Bugbot for commit cf28d76. This will update automatically on new commits. Configure here.

@pawelangelow pawelangelow force-pushed the fe/bugfix/fix-react-dom-warnings branch from 6a80c64 to 7a37365 Compare December 17, 2025 08:39
@pawelangelow pawelangelow marked this pull request as draft December 17, 2025 08:40
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.
@pawelangelow pawelangelow force-pushed the fe/bugfix/fix-react-dom-warnings branch from 7a37365 to f234c03 Compare December 17, 2025 08:44
Prevent duplicate refresh requests when keyboard event fires
while element still has focus during loading state.
@github-actions
Copy link
Contributor

Code Coverage - Frontend unit tests

St.
Category Percentage Covered / Total
🟢 Statements 82.82% 21103/25480
🟡 Branches 68.01% 8885/13064
🟡 Functions 78.04% 5784/7412
🟢 Lines 83.22% 20667/24834

Test suite run success

5486 tests passing in 704 suites.

Report generated by 🧪jest coverage report action from cf28d76

/>
</RiTooltip>
</div>
<RiTooltip content="Refresh Indexes">
Copy link
Contributor Author

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.

@pawelangelow pawelangelow self-assigned this Dec 17, 2025
@pawelangelow pawelangelow marked this pull request as ready for review December 17, 2025 10:19
</Text>
) : (
noItemsMessage
)}
Copy link

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.

Fix in Cursor Fix in Web

tabIndex={loading ? -1 : 0}
aria-disabled={loading}
aria-label="refresh indexes list"
onClick={handleRefresh}
Copy link

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.

Additional Locations (1)

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants