[IMP] Figure: Allow dragNdrop of multiple figures #7643
Conversation
|
|
hokolomopo
left a comment
hokolomopo
left a comment
There was a problem hiding this comment.
A bunch of small issues, and added some testing notes in the pad. But the feature is very cool :D
| import { Command, CommandResult } from "../../types/commands"; | ||
| import { UIPlugin } from "../ui_plugin"; | ||
|
|
||
| export class FigureUIPlugin extends UIPlugin { |
There was a problem hiding this comment.
I think we're trying to replace feature UI plugins by stores.
There was a problem hiding this comment.
In this case as there is no state and the plugin is only used to group the commands. I feel a plugin is a better choice according to this
packages/o-spreadsheet-engine/src/plugins/ui_stateful/carousel_ui.ts
Outdated
Show resolved
Hide resolved
packages/o-spreadsheet-engine/src/plugins/ui_stateful/carousel_ui.ts
Outdated
Show resolved
Hide resolved
| if (this.copiedData?.figureIds && this.copiedData.figureIds.length) { | ||
| const figureId = this.copiedData.figureIds[0]; |
There was a problem hiding this comment.
can we add all the copied charts inside the HTML ?
There was a problem hiding this comment.
The reason for this is that I want getClipboardTextAndImageContent to have matching HTML and Image content.
But I cannot add more image to the OS clipboard without a larger refactor of the clipboard to have multiple Image (which i plan to do another task for). So in the mean time, both method only copy the first image to the OS clipboard
| const figuresUI = selectedFiguresIds | ||
| .map((id) => this.getters.getFigure(sheetId, id)) | ||
| .filter(isDefined) | ||
| .map((f) => this.getters.getFigureUI(sheetId, f)); | ||
| const topLeft = rectUnion(...figuresUI); |
There was a problem hiding this comment.
I'd say it's the responsibility of the clipboard handlers to figure the top-left, not of the clipboard plugin
There was a problem hiding this comment.
I feel like I would rather leave it there compared to having this copied thrice in the three figure clipboard handlers (which would then need to act based on data they don't copy).
Task: 3323871
fdamhaut
force-pushed
the
master-multi-figure-select-flda
branch
from
42f3d68 to
0739df9
Compare
3 participants
Task: 3323871
Description:
description of this task, what is implemented and why it is implemented that way.
Task: TASK_ID
review checklist