-
Notifications
You must be signed in to change notification settings - Fork 10
Dash Canvas: add ability to drop widgets in from outside. #4183
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: develop
Are you sure you want to change the base?
Conversation
…e implemented via rglOptions escape hatch.
core/elem.ts
Outdated
| //------------------------ | ||
| // Implementation | ||
| //------------------------ | ||
| function normalizeArgs(args: any[], type: any) { |
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.
nice cleanup!
core/elem.ts
Outdated
| items?: ReactNode; | ||
|
|
||
| /** Equivalent to `items`, offered for code clarity when only one child is needed. */ | ||
| item?: Some<ReactNode>; |
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.
I could go either way here.
| * customize how the size of the dropping placeholder is calculated. The callback should | ||
| * return an object with optional properties indicating the desired width, height (in grid units), | ||
| * and offset (in pixels) of the dropping placeholder. | ||
| * If not provided, Hoist's own default logic will be used. |
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.
Last sentence is implicit.
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.
What is the meaning of false and void here for this type?
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.
I added more to that doc block to answer those questions.
lbwexler
left a comment
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.
can we have a separate PR for (a) the upgrade and proposed hoist changes and (b) the ability to drop widgets?
| * customize how the size of the dropping placeholder is calculated. The callback should | ||
| * return an object with optional properties indicating the desired width, height (in grid units), | ||
| * and offset (in pixels) of the dropping placeholder. | ||
| * If not provided, Hoist's own default logic will be used. |
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.
What is the meaning of false and void here for this type?
| maxRows: number; | ||
| droppable: boolean; | ||
| onDropDone: (viewModel: DashCanvasViewModel) => void; | ||
| draggedInView: DashCanvasItemState; |
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.
I find these name a bit confusing.
-
"droppable" might be "allowsDrop"
-
"draggedInView" might be 'droppedView'
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.
I have made this change: "droppable" might be "allowsDrop"
I'm not sure changing "draggedInView" to 'droppedView' is correct. "draggedInView" refers to a view that has been dragged in, but not yet dropped.
| } | ||
|
|
||
| onDropDragOver(evt: DragEvent): | ||
| | { |
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.
Need a type for the return -- if going to appear like this.
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.
done.
GridBackground from 'react-grid-layout/extras';
# Conflicts: # CHANGELOG.md
+ cancel drop if no RGL droppingItem
|
"Warning:(65, 11) Dependency npm:qs:6.14.0 is vulnerable CVE-2025-15284 7.5 arrayLimit bypass in bracket notation allows DoS via memory exhaustion Results powered by Mend.io"
| "numbro": "~2.5.0", | ||
| "onsenui": "~2.12.8", | ||
| "qs": "~6.14.0", | ||
| "qs": "~6.14.1", |
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.
Update qs dep to resolve this code analysis warning:
"Warning:(65, 11) Dependency npm:qs:6.14.0 is vulnerable CVE-2025-15284 7.5 arrayLimit bypass in bracket notation allows DoS via memory exhaustion Results powered by Mend.io"
| /** | ||
| * @param config - TabContainer configuration. | ||
| * @param [depth] - Depth in hierarchy of nested TabContainerModels. Not for application use. | ||
| * @param depth - Depth in hierarchy of nested TabContainerModels. Not for application use. |
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.
Unrelated linting change
| /** | ||
| * Begin an inline editing session. | ||
| * @param record - StoreRecord/ID to edit. If unspecified, the first selected StoreRecord | ||
| * @param opts.record - StoreRecord/ID to edit. If unspecified, the first selected StoreRecord |
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.
Unrelated linting change
…quently add wrap compact option, and use new dropConfig.onDragOver key.
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
DashCanvasWidgetWell CollapsibleFieldSet FieldsetCollapseButton
+ add support for intent & tooltip
|
For discussion - would something like |
Main Change:
Dash Canvas supports dragging in widgets from a "widget well".
review with new toolbox branch: https://github.com/xh/toolbox/tree/drag-onto-dash-canvas
Upgrades react-grid-layout to v2.2.2 to get their latest fixes.
Brings in 3 new hoist components:
Brings in 2 new tags:
Hoist P/R Checklist
Pull request authors: Review and check off the below. Items that do not apply can also be
checked off to indicate they have been considered. If unclear if a step is relevant, please leave
unchecked and note in comments.
developbranch as of last change.If your change is still a WIP, please use the "Create draft pull request" option in the split
button below to indicate it is not ready yet for a final review.