feat: Add keyboard shortcuts for quick actions in data browser #3073
Conversation
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. |
📝 WalkthroughWalkthroughAdds server-backed keyboard shortcuts: new settings UI and route, a KeyboardShortcutsManager library with validation/matching and server persistence, DataBrowser integration to act on shortcuts, minor TextInput prop/style additions, README docs and small settings styling. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (keyboard)
participant UI as Settings UI
participant Manager as KeyboardShortcutsManager
participant Server as ServerConfigStorage
participant Browser as DataBrowser
User->>UI: open Keyboard Shortcuts settings
UI->>Manager: getKeyboardShortcuts(appId)
Manager->>Server: read settings.keyboard.binding.*
Server-->>Manager: stored bindings / empty
Manager-->>UI: merged shortcuts (defaults + stored)
UI-->>User: render fields
User->>UI: edit & Save
UI->>Manager: saveKeyboardShortcuts(appId, shortcuts)
Manager->>Server: write settings.keyboard.binding.* (per shortcut)
Server-->>Manager: success
Manager-->>UI: save result
Note over Browser,Manager: runtime keyboard handling
Browser->>Manager: (on mount) getKeyboardShortcuts(appId)
Manager->>Server: read bindings
Manager-->>Browser: shortcuts
User->>Browser: press key
Browser->>Manager: matchesShortcut(event, shortcut)?
Manager-->>Browser: true/false
Browser-->>Browser: invoke action (reload/toggle) if matched
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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 |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![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 (4)
src/dashboard/Settings/KeyboardShortcutsSettings.react.js (1)
35-36: Unused refs can be removed.
reloadDataInputRefandtogglePanelsInputRefare created but never used in the component. Consider removing them unless they're planned for future functionality.- this.reloadDataInputRef = React.createRef(); - this.togglePanelsInputRef = React.createRef();And remove the ref props from TextInput components at lines 192 and 211.
src/lib/KeyboardShortcutsPreferences.js (3)
33-65: Consider validating fetched shortcuts before returning them.The method retrieves shortcuts from server storage but doesn't validate their structure before returning. If the server contains malformed data, invalid shortcuts could propagate to consumers.
Consider adding validation after line 50:
const shortcutName = key.replace('settings.keyboard.binding.', ''); - shortcuts[shortcutName] = value; + if (isValidShortcut(value)) { + shortcuts[shortcutName] = value; + } else { + console.warn(`Invalid shortcut for ${shortcutName}, using default`); + } }
116-142: Missing support for Meta/Cmd key.The validation only checks
ctrl,shift, andaltmodifiers. Mac users commonly use the Cmd (Meta) key for shortcuts, and this limitation prevents Cmd-based shortcuts.Additionally, the single-character key restriction (line 126) prevents special keys like "Enter", "Escape", or "Tab". If this is intentional to keep shortcuts simple, consider documenting this limitation in the function's JSDoc.
// Modifier keys are optional booleans if (shortcut.ctrl !== undefined && typeof shortcut.ctrl !== 'boolean') { return false; } if (shortcut.shift !== undefined && typeof shortcut.shift !== 'boolean') { return false; } if (shortcut.alt !== undefined && typeof shortcut.alt !== 'boolean') { return false; } + if (shortcut.meta !== undefined && typeof shortcut.meta !== 'boolean') { + return false; + }
162-173: Document that consumers should check event target.The function correctly matches keyboard events to shortcuts, but consumers must ensure shortcuts don't trigger when focus is in input fields, textareas, or contentEditable elements. Consider adding this to the JSDoc:
/** * Checks if a keyboard event matches a shortcut + * Note: Consumers should check event.target to prevent triggering + * shortcuts when typing in input fields (INPUT, TEXTAREA, contentEditable). * @param {KeyboardEvent} event - The keyboard event * @param {Object} shortcut - The shortcut object * @returns {boolean} True if matches */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
README.md(2 hunks)src/components/TextInput/TextInput.react.js(2 hunks)src/components/TextInput/TextInput.scss(1 hunks)src/dashboard/Dashboard.js(2 hunks)src/dashboard/DashboardView.react.js(1 hunks)src/dashboard/Data/Browser/DataBrowser.react.js(4 hunks)src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js(1 hunks)src/dashboard/Settings/KeyboardShortcutsSettings.react.js(1 hunks)src/dashboard/Settings/Settings.scss(1 hunks)src/lib/KeyboardShortcutsPreferences.js(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-13T19:47:18.023Z
Learnt from: mtrezza
Repo: parse-community/parse-dashboard PR: 0
File: :0-0
Timestamp: 2025-12-13T19:47:18.023Z
Learning: In Parse Dashboard DataBrowser, the panel header context menu uses the hardcoded 'objectId' field when filtering scripts via `getValidScripts` because the panel header represents the entire object, not a specific field. Field-specific script execution happens when right-clicking on individual table cells in BrowserCell.
Applied to files:
src/dashboard/Data/Browser/DataBrowser.react.jsREADME.md
📚 Learning: 2025-12-13T19:47:18.023Z
Learnt from: mtrezza
Repo: parse-community/parse-dashboard PR: 0
File: :0-0
Timestamp: 2025-12-13T19:47:18.023Z
Learning: In Parse Dashboard, checking for the `onEditSelectedRow` prop before showing script execution options is intentional and consistent across both DataBrowser and BrowserCell components. This prop indicates the browser is in an editable state, preventing scripts from being shown in read-only views.
Applied to files:
README.md
🧬 Code graph analysis (2)
src/dashboard/Data/Browser/DataBrowser.react.js (1)
src/lib/KeyboardShortcutsPreferences.js (2)
KeyboardShortcutsManager(22-109)matchesShortcut(162-173)
src/dashboard/Dashboard.js (1)
src/dashboard/Settings/KeyboardShortcutsSettings.react.js (1)
KeyboardShortcutsSettings(20-239)
⏰ 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)
- GitHub Check: Node 18
- GitHub Check: Docker linux/amd64
🔇 Additional comments (16)
src/components/TextInput/TextInput.scss (1)
24-27: ✅ LGTM – placeholder styling follows established patterns.The addition of explicit placeholder color and opacity improves visibility while remaining consistent with the existing
:focusstate behavior (opacity: 0 when focused). The styling choice is reasonable for UX.src/dashboard/Settings/Settings.scss (1)
12-24: LGTM - Button group styling is appropriate.The flexbox layout with centering and gap works well for button groups. The
!importantdeclarations on child divs are likely needed to override FormButton's default styles.src/dashboard/Settings/DashboardSettings/DashboardSettings.react.js (1)
477-477: LGTM - Documentation text accurately reflects the new feature.The updated text correctly includes Keyboard Shortcuts in the list of server-storable settings.
src/components/TextInput/TextInput.react.js (1)
72-73: LGTM - Clean API extension for focus handling and input length restriction.The new
onFocusandmaxLengthprops are properly forwarded to the input element and documented via PropTypes.src/dashboard/Dashboard.js (1)
58-58: LGTM - Import and route registration follow existing patterns.The new settings route is properly integrated with the existing routing structure.
src/dashboard/DashboardView.react.js (1)
215-218: LGTM - Navigation entry correctly added to settings sections.The Keyboard Shortcuts entry follows the existing pattern and is logically positioned in the settings navigation.
src/dashboard/Data/Browser/DataBrowser.react.js (2)
265-277: LGTM - Keyboard shortcuts loading is properly implemented.The async componentDidMount correctly loads shortcuts from the server with appropriate error handling. The warning log on failure is a good fallback behavior.
863-890: LGTM - Custom shortcut handling is well-structured.The implementation correctly:
- Guards against null shortcuts
- Checks class-specific conditions before toggling panels
- Prevents default behavior only when shortcuts are handled
- Uses the
matchesShortcututility for consistent key comparisonsrc/dashboard/Settings/KeyboardShortcutsSettings.react.js (2)
39-44: LGTM - Component initialization is correct.The manager is properly initialized with the app context, and shortcuts are loaded on mount with appropriate guards.
80-117: LGTM - Save logic with proper validation.The implementation correctly:
- Creates null shortcuts when fields are empty (allowing shortcuts to be disabled)
- Validates shortcuts only when set
- Performs case-insensitive duplicate detection
- Handles errors gracefully with user feedback
src/lib/KeyboardShortcutsPreferences.js (6)
1-10: LGTM!Standard copyright header and necessary import are in place.
22-26: LGTM!The constructor properly initializes the ServerConfigStorage dependency. The design allows methods to accept
appIdparameters while maintaining theappcontext from construction, enabling cross-app operations if needed.
98-100: LGTM!Clean delegation to the save method with default shortcuts.
106-108: LGTM!Clean delegation to the underlying storage check.
149-154: LGTM!The function correctly creates a valid shortcut object consistent with the validation rules.
14-17: Keyboard shortcuts for 'r' and 'p' keys are already protected from interfering with text input, but contentEditable elements are not covered.The
handleKeymethod in DataBrowser already filters keyboard events when focus is on INPUT, TEXTAREA, or SELECT elements (lines 620-628), so the plain 'r' and 'p' shortcuts won't trigger during normal text editing in these input fields.However, the check does not account for contentEditable elements. If your application uses contentEditable divs or similar elements, keyboard shortcuts could still fire while users are editing content. Consider extending the check to also exclude contentEditable elements:
target.contentEditable === 'true'.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/lib/KeyboardShortcutsPreferences.js (1)
78-103: Validation added before persisting - past review addressed.The validation loop at lines 83-88 now validates all shortcuts before any writes, preventing partial saves of invalid data.
🧹 Nitpick comments (3)
src/lib/KeyboardShortcutsPreferences.js (1)
139-142: Consider validating key character range to match documentation.The comment states "only single-character keys are supported (a-z, 0-9)" but the code accepts any single character. This could allow special characters like
!,@, etc.If strict validation is desired:
// Must have a key property that is a single character - if (!shortcut.key || typeof shortcut.key !== 'string' || shortcut.key.length !== 1) { + if (!shortcut.key || typeof shortcut.key !== 'string' || !/^[a-z0-9]$/i.test(shortcut.key)) { return false; }src/dashboard/Settings/KeyboardShortcutsSettings.react.js (2)
83-98: Validation logic will never show error messages - consider simplifying.The validation at lines 90-98 is effectively dead code.
createShortcutreturnsnullfor invalid input, andisValidShortcut(null)returnstrue. So the conditionshortcuts.X && !isValidShortcut(shortcuts.X)can never be true since invalid input becomesnullwhich is falsy.Since
TextInputhasmaxLength={1}, the UI prevents invalid input. Consider removing the unreachable validation or adding explicit validation before callingcreateShortcut:async handleSave() { if (!this.context || !this.manager) { return; } + // Validate input strings before creating shortcuts + const reloadKey = this.state.dataBrowserReloadData; + const toggleKey = this.state.dataBrowserToggleInfoPanels; + + if (reloadKey && reloadKey.length !== 1) { + this.showNote('Invalid key for "Reload Data". Please enter a single character.', true); + return; + } + if (toggleKey && toggleKey.length !== 1) { + this.showNote('Invalid key for "Toggle Panels". Please enter a single character.', true); + return; + } + // Create shortcut objects from the key strings const shortcuts = { - dataBrowserReloadData: this.state.dataBrowserReloadData ? createShortcut(this.state.dataBrowserReloadData) : null, - dataBrowserToggleInfoPanels: this.state.dataBrowserToggleInfoPanels ? createShortcut(this.state.dataBrowserToggleInfoPanels) : null, + dataBrowserReloadData: reloadKey ? createShortcut(reloadKey) : null, + dataBrowserToggleInfoPanels: toggleKey ? createShortcut(toggleKey) : null, }; - // Validate shortcuts (only if they are set) - if (shortcuts.dataBrowserReloadData && !isValidShortcut(shortcuts.dataBrowserReloadData)) { - this.showNote('Invalid key for "Reload Data". Please enter a valid key.', true); - return; - } - - if (shortcuts.dataBrowserToggleInfoPanels && !isValidShortcut(shortcuts.dataBrowserToggleInfoPanels)) { - this.showNote('Invalid key for "Toggle Panels". Please enter a valid key.', true); - return; - } - // Check for duplicates (only if both are set)
188-196: Consider binding handlers in constructor instead of render.Using
.bind(this)in render creates new function instances on each render. While minor, binding in the constructor is more efficient.In constructor:
this.handleFieldChange = this.handleFieldChange.bind(this); this.handleInputFocus = this.handleInputFocus.bind(this); this.handleSave = this.handleSave.bind(this); this.handleReset = this.handleReset.bind(this);Then in render use:
- onChange={this.handleFieldChange.bind(this, 'dataBrowserReloadData')} - onFocus={this.handleInputFocus.bind(this)} + onChange={(value) => this.handleFieldChange('dataBrowserReloadData', value)} + onFocus={this.handleInputFocus}Also applies to: 206-214
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/dashboard/Settings/KeyboardShortcutsSettings.react.js(1 hunks)src/lib/KeyboardShortcutsPreferences.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/dashboard/Settings/KeyboardShortcutsSettings.react.js (2)
src/lib/KeyboardShortcutsPreferences.js (5)
KeyboardShortcutsManager(22-121)createShortcut(164-169)isValidShortcut(130-157)DEFAULT_SHORTCUTS(14-17)DEFAULT_SHORTCUTS(14-17)src/dashboard/Data/Browser/Notification.react.js (1)
Notification(13-80)
src/lib/KeyboardShortcutsPreferences.js (1)
src/lib/ServerConfigStorage.js (1)
ServerConfigStorage(14-202)
⏰ 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 (9)
src/lib/KeyboardShortcutsPreferences.js (4)
14-17: Well-defined default shortcuts structure.Clean definition of default shortcuts with explicit modifier key states. This provides good documentation of the expected structure.
33-70: LGTM! Robust shortcut loading with validation and fallback.The method correctly handles disabled server config, validates incoming data, logs warnings for invalid shortcuts, and gracefully falls back to defaults on errors.
164-169: LGTM!Simple factory function with proper null handling for invalid input.
179-190: LGTM! Good keyboard event matching logic.The double-bang coercion for modifier keys (e.g.,
!!event.ctrlKey === !!shortcut.ctrl) correctly handles cases where modifier properties might be undefined or have default false values on the shortcut object.src/dashboard/Settings/KeyboardShortcutsSettings.react.js (5)
20-35: LGTM!Clean initialization with appropriate state defaults for the form.
117-130: LGTM!Clean reset flow that properly persists defaults and reloads UI state.
146-156: LGTM!Loading state properly shows skeleton UI with the toolbar.
158-170: LGTM!Clear error state when server configuration is not enabled, with actionable message for the user.
218-229: LGTM!Good UX with Save disabled when unchanged, and Reset always available.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/dashboard/DashboardView.react.js (1)
43-50: Consider simplifying getCurrentRoute().The
getCurrentRoute()helper adds defensive fallback logic, butstate.routeshould always be populated byonRouteChanged()which runs on both mount and update. The fallback extraction duplicates the logic from line 36.Since the component lifecycle ensures
state.routeis set, you could simplify this to just returnstate.routedirectly, or inline the helper at line 331.- getCurrentRoute() { - // If state.route is set, use it; otherwise extract from current location - if (this.state.route) { - return this.state.route; - } - const path = this.props.location?.pathname ?? window.location.pathname; - return path.split('apps')[1]?.split('/')[2] || ''; - } + getCurrentRoute() { + return this.state.route; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/dashboard/Dashboard.js(2 hunks)src/dashboard/DashboardView.react.js(3 hunks)src/dashboard/Settings/KeyboardShortcutsSettings.react.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/dashboard/Settings/KeyboardShortcutsSettings.react.js
🧰 Additional context used
🧬 Code graph analysis (1)
src/dashboard/Dashboard.js (2)
src/dashboard/Settings/SettingsData.react.js (1)
SettingsData(12-56)src/dashboard/Settings/KeyboardShortcutsSettings.react.js (1)
KeyboardShortcutsSettings(20-220)
⏰ 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 (5)
src/dashboard/Dashboard.js (2)
58-58: LGTM!The import is correctly added for the new keyboard shortcuts settings component.
236-248: Verify the architectural difference is intentional.The
keyboard-shortcutsroute is placed outside the<SettingsData />wrapper, unlike other settings routes (dashboard, security, general, etc.). This meansKeyboardShortcutsSettingswon't have access to theinitialFieldsandsaveChangescontext provided bySettingsData.Based on the relevant code snippet showing that
KeyboardShortcutsSettingsusesKeyboardShortcutsManagerinstead, this separation appears intentional. However, this creates an architectural inconsistency where different settings routes use different persistence mechanisms.Please confirm this design is intentional and consider documenting why keyboard shortcuts use a separate persistence approach.
src/dashboard/DashboardView.react.js (3)
36-36: LGTM!Good defensive improvement using optional chaining to handle cases where
this.props.locationmight be undefined.
224-227: LGTM!The new "Keyboard Shortcuts" settings item is correctly added to the settings navigation with the appropriate link.
329-332: LGTM!The validation logic correctly includes
settingsSectionsin the route validation, which is necessary for the new keyboard shortcuts route. UsinggetCurrentRoute()provides consistent route extraction.
# [8.2.0-alpha.7](8.2.0-alpha.6...8.2.0-alpha.7) (2025-12-15) ### Features * Add keyboard shortcuts for quick actions in data browser ([#3073](#3073)) ([858d0cc](858d0cc))
|
🎉 This change has been released in version 8.2.0-alpha.7 |
2 participants
New Pull Request Checklist
Summary by CodeRabbit
New Features
Enhancements
Documentation
Style
✏️ Tip: You can customize this high-level summary in your review settings.