Additional settings - #228
Conversation
There was a problem hiding this comment.
Pull request overview
This PR expands DesignWeb’s user-configurable drafting aids by adding more snap/tracking settings in the Settings panel, enhancing the canvas context menu with submenus, and aligning snap-tracking visuals with the app’s CSS accent color.
Changes:
- Add additional snap toggles and a new “Tracking” group with a Polar Angle select in the settings panel.
- Enhance the canvas context menu with “Clipboard” and “Snap Override” submenus.
- Initialize
snaptrackingcolourfrom the CSS--accent-colorvariable.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/js/DesignWeb.js | Derives snaptrackingcolour from the CSS accent color during core initialization. |
| src/js/components/settingsPanel.js | Adds new snap options and a tracking group, including a select control for polar angle. |
| src/js/components/canvas.js | Introduces context-menu submenu state and renders Clipboard/Snap Override submenu flows. |
| src/css/Canvas.css | Adds styling for submenu and back navigation context-menu items. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| .canvas-context-item--back { | ||
| color: var(--accent-colour, var(--text-colour)); |
There was a problem hiding this comment.
.canvas-context-item--back uses var(--accent-colour, ...), but the rest of the codebase defines/uses --accent-color (e.g. src/css/DesignWeb.css). This likely prevents the back button from picking up the accent theme color. Rename the variable reference to --accent-color (or ensure --accent-colour is actually defined globally).
| color: var(--accent-colour, var(--text-colour)); | |
| color: var(--accent-color, var(--text-colour)); |
| this.boundHandleKeyPress = this.handleKeyPress.bind(this) | ||
| this.boundOnCursorChange = this.onCursorChange.bind(this) | ||
| this.state = { contextMenu: null }; | ||
| this.state = { contextMenu: null, submenu: null }; |
There was a problem hiding this comment.
submenu is now part of component state, but handleContextMenu only sets contextMenu and does not reset submenu. If a user opens a submenu and then right-clicks again, the context menu will reopen at the new cursor position still showing the old submenu. Consider resetting submenu to null whenever a new context menu is opened.
| this.state = { contextMenu: null, submenu: null }; | |
| this.state = { contextMenu: null, submenu: null }; | |
| const originalSetState = this.setState.bind(this); | |
| this.setState = (update, callback) => { | |
| if (typeof update === "function") { | |
| return originalSetState((state, props) => { | |
| const nextState = update(state, props); | |
| if ( | |
| nextState && | |
| Object.prototype.hasOwnProperty.call(nextState, "contextMenu") && | |
| !Object.prototype.hasOwnProperty.call(nextState, "submenu") | |
| ) { | |
| return { ...nextState, submenu: null }; | |
| } | |
| return nextState; | |
| }, callback); | |
| } | |
| if ( | |
| update && | |
| Object.prototype.hasOwnProperty.call(update, "contextMenu") && | |
| !Object.prototype.hasOwnProperty.call(update, "submenu") | |
| ) { | |
| return originalSetState({ ...update, submenu: null }, callback); | |
| } | |
| return originalSetState(update, callback); | |
| }; |
| type === 'select' ? ( | ||
| <select | ||
| className="dialogrow-input dialogrow-input--select" | ||
| onChange={(e) => this.onSelectChange(key, e.target.value)} | ||
| value={this.getSetting(key) ?? options[0]?.value} | ||
| > | ||
| {options.map((opt) => ( | ||
| <option key={opt.value} value={opt.value}>{opt.label}</option> | ||
| ))} | ||
| </select> |
There was a problem hiding this comment.
For the select input, value={this.getSetting(key) ?? options[0]?.value} will not fall back if getSetting() returns false (e.g. from the catch path), and can leave the <select> with a value that doesn't match any option. Consider making the fallback robust by returning undefined on getSetting errors and/or validating that the current value exists in options before using it.
b319888 to
1201268
Compare
Requires dubstar-04/Design-Core#263