feat: Add Settings menu to scroll info panel to top when browsing through rows #2937
Conversation
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughThis update introduces hierarchical submenu support to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BrowserMenu
participant Submenu
participant ParentMenu
User->>BrowserMenu: Clicks root menu
BrowserMenu->>BrowserMenu: Opens menu
User->>BrowserMenu: Mouse enters submenu entry
BrowserMenu->>Submenu: Opens submenu (left/right based on viewport)
Submenu->>BrowserMenu: Receives parentClose callback
User->>Submenu: Selects submenu item
Submenu->>BrowserMenu: Calls parentClose to close both menus
sequenceDiagram
participant DataBrowser
participant BrowserToolbar
participant User
participant AggregationPanel
User->>BrowserToolbar: Opens "Settings" > "Info Panel"
User->>BrowserToolbar: Toggles "Scroll to top"
BrowserToolbar->>DataBrowser: Calls toggleScrollToTop()
DataBrowser->>AggregationPanel: On data/selection change, if scrollToTop, scrolls panel to top
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error code ERR_SSL_WRONG_VERSION_NUMBER Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
mtrezza
changed the title
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/dashboard/Data/Browser/BrowserToolbar.react.js (1)
448-471: Well-implemented Settings menu with scroll toggle.The nested menu structure is clean and follows established patterns. The conditional check icon rendering provides good visual feedback for the toggle state.
Consider using a CSS variable or theme color instead of the hardcoded fill color:
- fill="#ffffffff" + fill="white"Also, to prevent layout shift when the check icon appears/disappears, you could reserve space:
text={ <span> - {scrollToTop && ( - <Icon - name="check" - width={12} - height={12} - fill="white" - className="menuCheck" - /> - )} + <Icon + name="check" + width={12} + height={12} + fill="white" + className="menuCheck" + style={{ opacity: scrollToTop ? 1 : 0 }} + /> Scroll to top </span> }src/components/BrowserMenu/BrowserMenu.react.js (2)
56-69: Consider extracting submenu positioning logicThe inline style calculation for submenu positioning is complex and could benefit from extraction into a separate function for better readability and testability.
+ getSubmenuStyles() { + if (!this.props.parentClose) return { minWidth: this.wrapRef.current.clientWidth }; + + return { + minWidth: this.wrapRef.current.clientWidth, + top: 0, + left: this.state.openToLeft ? 0 : `${this.wrapRef.current.clientWidth - 3}px`, + transform: this.state.openToLeft ? 'translateX(calc(-100% + 3px))' : undefined, + }; + } <div className={...} - style={{ - minWidth: this.wrapRef.current.clientWidth, - ...(isSubmenu - ? { - top: 0, - left: this.state.openToLeft - ? 0 - : `${this.wrapRef.current.clientWidth - 3}px`, - transform: this.state.openToLeft - ? 'translateX(calc(-100% + 3px))' - : undefined, - } - : {}), - }} + style={this.getSubmenuStyles()} >
117-125: Optimize submenu detection logicThe submenu detection logic on line 118 could be expensive as it converts children to array and iterates on every render. Consider memoizing this check or computing it once.
+ hasSubmenu() { + return React.Children.toArray(this.props.children).some( + c => React.isValidElement(c) && c.type === BrowserMenu + ); + } - {isSubmenu && - React.Children.toArray(this.props.children).some(c => React.isValidElement(c) && c.type === BrowserMenu) && ( + {isSubmenu && this.hasSubmenu() && (For better performance, consider using
useMemoif converting to a functional component in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/icons/script-solid.svgis excluded by!**/*.svgsrc/icons/script.svgis excluded by!**/*.svg
📒 Files selected for processing (8)
src/components/BrowserMenu/BrowserMenu.react.js(4 hunks)src/components/BrowserMenu/BrowserMenu.scss(1 hunks)src/components/BrowserMenu/MenuItem.react.js(1 hunks)src/components/Icon/Icon.react.js(2 hunks)src/components/Icon/Icon.scss(1 hunks)src/dashboard/Data/Browser/BrowserToolbar.react.js(4 hunks)src/dashboard/Data/Browser/DataBrowser.react.js(6 hunks)src/dashboard/Data/Views/Views.react.js(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
src/components/Icon/Icon.scss (1)
Learnt from: mtrezza
PR: #2726
File: src/components/ContextMenu/ContextMenu.react.js:56-74
Timestamp: 2025-05-24T18:24:15.988Z
Learning: In the Parse Dashboard ContextMenu component, the one-time positioning calculation using hasPositioned ref works well even with window resizing and movement, so additional window resize event listeners are not necessary for menu positioning.
src/dashboard/Data/Browser/DataBrowser.react.js (1)
Learnt from: mtrezza
PR: #2828
File: src/dashboard/Data/Browser/Browser.react.js:1605-1607
Timestamp: 2025-05-27T12:09:47.644Z
Learning: In script execution dialogs in Parse Dashboard (specifically the confirmExecuteScriptRows method in src/dashboard/Data/Browser/Browser.react.js), individual setState calls to update processedScripts counter should be kept as-is rather than batched, because this provides real-time progress feedback to users in the dialog UI.
src/components/BrowserMenu/BrowserMenu.react.js (1)
Learnt from: mtrezza
PR: #2726
File: src/components/ContextMenu/ContextMenu.react.js:56-74
Timestamp: 2025-05-24T18:24:15.988Z
Learning: In the Parse Dashboard ContextMenu component, the one-time positioning calculation using hasPositioned ref works well even with window resizing and movement, so additional window resize event listeners are not necessary for menu positioning.
🧬 Code Graph Analysis (1)
src/dashboard/Data/Browser/BrowserToolbar.react.js (3)
src/components/BrowserMenu/BrowserMenu.react.js (1)
BrowserMenu(15-131)src/components/BrowserMenu/MenuItem.react.js (1)
MenuItem(11-43)src/components/Icon/Icon.react.js (1)
Icon(12-29)
⏰ 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). (1)
- GitHub Check: Docker linux/amd64
🔇 Additional comments (11)
src/components/BrowserMenu/BrowserMenu.scss (1)
93-123: LGTM! Well-structured submenu styling.The new CSS classes for submenu functionality are well-designed with consistent styling patterns. The dynamic positioning support through different border-radius values for left/right placement is a nice touch, and the hover effects for the submenu arrow maintain visual consistency with existing menu elements.
src/dashboard/Data/Views/Views.react.js (1)
343-343: Good security enhancement!Adding
noopenerto therelattribute alongsidenoreferrerprevents the new page from accessing thewindow.openerproperty, which is a security best practice when opening links withtarget="_blank".src/components/Icon/Icon.scss (1)
10-14: LGTM! Clean icon styling for menu check marks.The
.menuCheckclass provides appropriate spacing and alignment for check icons in menu items. The negative bottom margin is a good technique for fine-tuning vertical alignment.src/dashboard/Data/Browser/BrowserToolbar.react.js (2)
85-86: LGTM! Props properly added for scroll functionality.The new
scrollToTopandtoggleScrollToTopprops are well-defined and integrate cleanly with the existing toolbar component structure.
429-429: Good semantic improvement with icon change.Changing from
"gear-solid"to"script-solid"for the Script menu makes more semantic sense and better represents the menu's functionality.src/dashboard/Data/Browser/DataBrowser.react.js (5)
110-110: Good default value for scroll behavior.Initializing
scrollToToptotruemakes sense as the default behavior, ensuring the info panel scrolls to top on data changes unless explicitly disabled by the user.
134-134: Clean state management implementation.The
toggleScrollToTopmethod follows React best practices by using the functional setState pattern to toggle the boolean value. Method binding in the constructor is also appropriate.Also applies to: 671-673
137-137: Proper ref usage for scroll control.The
aggregationPanelRefis correctly created and applied to the container div, providing direct access to the DOM element for scroll manipulation.Also applies to: 923-926
222-231: Well-structured scroll-to-top logic.The
componentDidUpdateimplementation has comprehensive condition checking:
- Detects changes in aggregation data or selected object
- Ensures panel visibility before scrolling
- Validates ref existence before DOM manipulation
- Respects user preference via
scrollToTopstateThe synchronous
scrollTop = 0operation is appropriate for this use case.
973-974: Props properly passed to toolbar component.The
scrollToTopstate andtoggleScrollToTopmethod are correctly passed as props to enable the toolbar's Settings menu functionality.src/components/Icon/Icon.react.js (1)
18-20: Well-implemented className handlingGood implementation that checks if the className exists in the CSS module before applying it, with a fallback to the raw className. This provides flexibility while maintaining CSS module benefits.
| const width = this.wrapRef.current.clientWidth; | ||
| const openToLeft = rect.right + width > window.innerWidth; | ||
| this.setState({ open: true, openToLeft }); | ||
| this.props.setCurrent?.(null); |
There was a problem hiding this comment.
Add missing setCurrent to PropTypes
The setCurrent prop is used in the component but not defined in PropTypes. This should be documented for better type safety and clarity.
BrowserMenu.propTypes = {
icon: PropTypes.string.describe('The name of the icon to place in the menu.'),
title: PropTypes.string.isRequired.describe('The title text of the menu.'),
children: PropTypes.oneOfType([PropTypes.arrayOf(PropTypes.node), PropTypes.node]).describe(
'The contents of the menu when open. It should be a set of MenuItem and Separator components.'
),
parentClose: PropTypes.func.describe(
'Closes the parent menu when a nested menu item is selected.'
),
+ setCurrent: PropTypes.func.describe(
+ 'Function to set the current menu state in the parent component.'
+ ),
+ active: PropTypes.bool.describe(
+ 'Whether the menu item is currently active.'
+ ),
+ disabled: PropTypes.bool.describe(
+ 'Whether the menu item is disabled.'
+ ),
};Also applies to: 108-108, 133-142
🤖 Prompt for AI Agents
In src/components/BrowserMenu/BrowserMenu.react.js around lines 103, 108, and
133-142, the setCurrent prop is used but missing from the component's PropTypes
definition. Add setCurrent to the PropTypes object, specifying its expected type
(likely a function), to improve type safety and documentation of the component's
props.
| onClick={handleClick} | ||
| onMouseDown={handleClick} // This is needed - onClick alone doesn't work in this context |
There was a problem hiding this comment.
Potential double execution of click handler
Setting both onClick and onMouseDown to the same handler could cause the function to execute twice in some scenarios. If onClick alone doesn't work due to event propagation issues, consider using only onMouseDown or investigating the root cause.
- onClick={handleClick}
- onMouseDown={handleClick} // This is needed - onClick alone doesn't work in this context
+ onMouseDown={handleClick} // Using onMouseDown due to event propagation requirements📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onClick={handleClick} | |
| onMouseDown={handleClick} // This is needed - onClick alone doesn't work in this context | |
| onMouseDown={handleClick} // Using onMouseDown due to event propagation requirements |
🤖 Prompt for AI Agents
In src/components/BrowserMenu/MenuItem.react.js at lines 32-33, the click
handler is assigned to both onClick and onMouseDown, which may cause it to
execute twice. To fix this, remove the onClick handler and keep only onMouseDown
if onClick alone doesn't work, or investigate event propagation issues to ensure
the handler runs only once per user interaction.
| style={{ | ||
| position: 'relative', | ||
| zIndex: 9999, | ||
| cursor: 'pointer' | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move inline styles to CSS for better maintainability
The inline styles should be moved to the BrowserMenu.scss file to maintain consistency and improve maintainability. The high z-index value (9999) could also cause stacking issues with other UI elements.
Add to BrowserMenu.scss:
.item {
position: relative;
z-index: 10; // Use a more reasonable z-index
cursor: pointer;
}Then update the component:
- style={{
- position: 'relative',
- zIndex: 9999,
- cursor: 'pointer'
- }}🤖 Prompt for AI Agents
In src/components/BrowserMenu/MenuItem.react.js around lines 34 to 38, move the
inline styles for position, zIndex, and cursor into the BrowserMenu.scss file by
creating a CSS class named "item" with position relative, z-index 10, and cursor
pointer. Then update the component to use this CSS class instead of inline
styles to improve maintainability and avoid potential stacking issues from the
high z-index value.
# [7.3.0-alpha.27](7.3.0-alpha.26...7.3.0-alpha.27) (2025-07-24) ### Features * Add Settings menu to scroll info panel to top when browsing through rows ([#2937](#2937)) ([f339cb8](f339cb8))
|
🎉 This change has been released in version 7.3.0-alpha.27 |
# [7.3.0](7.2.1...7.3.0) (2025-08-01) ### Bug Fixes * Changing "Relative dates" option of saved filter does not enable save button ([#2947](#2947)) ([4f4977d](4f4977d)) * Class object counters in sidebar not updating ([#2950](#2950)) ([0f1920b](0f1920b)) * Clicking linked pointer with Cmd key in view table doesn't open page in new browser tab ([#2902](#2902)) ([101b194](101b194)) * Fails to generate MFA code with CLI command `parse-dashboard --createMFA` ([#2883](#2883)) ([544df1f](544df1f)) * Gracefully fail when trying to get new features in latest version of dashboard ([#2880](#2880)) ([1969a0e](1969a0e)) * Header checkbox in data browser does not indicate when a few rows are selected ([#2957](#2957)) ([e4ab666](e4ab666)) * Hyperlink in Views table ignores `urlQuery` key ([#2926](#2926)) ([c5eedf4](c5eedf4)) * Incorrect table cell width in App Settings table ([#2933](#2933)) ([d46765b](d46765b)) * Info panel scroll-to-top setting not persistent across dashboard sessions ([#2938](#2938)) ([2b78087](2b78087)) * Invalid clipboard content for multi-cell copy in data browser ([#2882](#2882)) ([22a2065](22a2065)) * Legacy filters without `filterId` cannot be deleted in data browser ([#2946](#2946)) ([65df9d6](65df9d6)) * Legacy filters without `filterId` do not appear in sidebar ([#2945](#2945)) ([fde3769](fde3769)) * Modal text input can be resized smaller than its cell in Safari browser ([#2930](#2930)) ([82a0cdc](82a0cdc)) * Move settings button on data browser toolbar for better UI ([#2940](#2940)) ([c473ce6](c473ce6)) * Pagination footer bar hides rows in data browser ([#2879](#2879)) ([6bc2da8](6bc2da8)) * Race condition on info panel request shows info panel data not corresponding to selected cell ([#2909](#2909)) ([6f45bb3](6f45bb3)) * Saved legacy filter in data browser cannot be deleted or cloned ([#2944](#2944)) ([15da90d](15da90d)) * Saved legacy filter with classname in query cannot be deleted ([#2948](#2948)) ([05ee5b3](05ee5b3)) * Selected text in info panel cannot be copied using Ctrl+C ([#2951](#2951)) ([0164c19](0164c19)) * Views not sorted alphabetically in sidebar ([#2943](#2943)) ([4c81fe4](4c81fe4)) * Warning dialog is shown after executing script on selected rows ([#2899](#2899)) ([027f1ed](027f1ed)) ### Features * Add additional values in info panel key-value element ([#2904](#2904)) ([a8f110e](a8f110e)) * Add AI agent for natural language interaction with Parse Server ([#2954](#2954)) ([32bd6e8](32bd6e8)) * Add clipboard icon to copy value of key-value element in info panel ([#2871](#2871)) ([7862c42](7862c42)) * Add Cloud Function as data source for views with optional text or file upload ([#2939](#2939)) ([f5831c7](f5831c7)) * Add column freezing in data browser ([#2877](#2877)) ([29f4a88](29f4a88)) * Add custom data views with aggregation query ([#2888](#2888)) ([b1679db](b1679db)) * Add environment variable support for AI agent configuration ([#2956](#2956)) ([2ac9e7e](2ac9e7e)) * Add hyperlink support in Views table ([#2925](#2925)) ([06cfc11](06cfc11)) * Add inclusive date filters "is on or after", "is on or before" in data browser ([#2929](#2929)) ([c8d621b](c8d621b)) * Add quick-add button to array parameter in Cloud Config ([#2866](#2866)) ([e98ccb2](e98ccb2)) * Add row number column to data browser ([#2878](#2878)) ([c0aa407](c0aa407)) * Add Settings menu to scroll info panel to top when browsing through rows ([#2937](#2937)) ([f339cb8](f339cb8)) * Add support for "not equal to" filter for Boolean values in data browser and analytics explorer ([#2914](#2914)) ([d55b89c](d55b89c)) * Add support for `Image` type in View table to display images ([#2952](#2952)) ([6a6b1f0](6a6b1f0)) * Add type mismatch warning when quick-adding entry to Cloud Config array parameter ([#2875](#2875)) ([bb1837f](bb1837f)) * Add view edit icon to views list in sidebar ([#2901](#2901)) ([96e33b9](96e33b9)) * Allow editing filter without loading data in data browser ([#2949](#2949)) ([9623580](9623580)) * Allow editing saved filters in data browser ([#2942](#2942)) ([daaccaa](daaccaa)) * Allow freeform text view resizing in modal dialogs ([#2910](#2910)) ([1399162](1399162)) * Persist info panel visibility when navigating across classes in data browser ([#2908](#2908)) ([1a3610a](1a3610a)) * Prefetch info panel data with config options `prefetchObjects` and `prefetchStale` ([#2915](#2915)) ([54a8156](54a8156)) * Warn when leaving data browser page with selected rows ([#2887](#2887)) ([206ead1](206ead1)) ### Performance Improvements * Add config option `enableResourceCache` to cache dashboard resources locally for faster loading in additional browser tabs ([#2920](#2920)) ([41a4963](41a4963))
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
noopenerto link attributes.Style