Skip to content

Copy, paste and duplicate the selected chart, picture or shape - #1577

Open
marcoarnulfo wants to merge 1 commit into
storytold:mainfrom
marcoarnulfo:fix/copy-paste-charts
Open

marcoarnulfo wants to merge 1 commit into
storytold:mainfrom
marcoarnulfo:fix/copy-paste-charts

Conversation

@marcoarnulfo

Copy link
Copy Markdown
Contributor

Fixes #1563.

With a chart selected, the grid's Cmd/Ctrl+C handler still copied the cells of the cell selection and Cmd/Ctrl+V pasted the clipboard text into them, so no copy of the chart ever appeared, and there was no command to duplicate a floating object. This adds object.duplicate {kind, id, dx?, dy?}, which clones a chart, picture or shape on the active sheet with a fresh id and the same series and formatting (same ranges, so the copy is independent but plots the same data), offset 12 pt right and down, as one undoable step. It builds on the delete-the-selected-object work in #213.

In the grid, Copy with an object selected remembers the object and ends the cells' copy mode. Paste duplicates it, each paste a step further, with the copy selected, as long as the system clipboard still holds what it held at the copy, so text copied in another app afterwards still pastes into the cells. A few details:

  • Where the clipboard can't be read on request (web), the first paste's text is taken as the copy's.
  • With no text on the clipboard, egui-winit sends neither a Paste event nor the key press for Cmd/Ctrl+V, so the key's release pastes the object when no Paste event came before it.
  • If the copied object has been deleted or undone in the meantime, Paste falls back to the text.
  • The ribbon and menu Paste go through the same path, and Cmd/Ctrl+D duplicates the selected object instead of filling down, as in Excel.
  • Moving the cell cursor from the keyboard or starting to type deselects the object, so Copy and Cmd/Ctrl+D act on cells again.

Left out, happy to add or switch: pasting on a different sheet still pastes cells (the series would keep pointing at the original sheet, which felt like its own decision); Cut, the ribbon Copy button and the context menus still act on the cells. If #470 lands first I'm glad to rebase onto its ObjRef selection.

One note on CI: adding a command shifts which commands command_sequence_fuzz picks for its fixed seeds, and seed 0 now hits an existing spill issue that doesn't involve the new command (the three steps home.insertSheet, cell.set C14 =TRANSPOSE(B18:B24), range.fill =D14:E18 into E13:F13 give #SPILL! incrementally and 0 after a full recalc, on main too). It looks like the contested-spill case in #964 / #1190, which #984 addresses; happy to rebase once that lands.

Validation:

  • clipboard::copy_paste_and_cmd_d_duplicate_the_selected_chart (real Copy/Paste events and Cmd+D through egui_kittest): fails on main (Copy writes the cells to the clipboard, no second chart), passes here, also alone with --test-threads=1
  • clipboard::a_copied_chart_pastes_where_the_system_clipboard_cannot_be_read, ..._when_the_system_clipboard_holds_no_text, moving_in_the_grid_deselects_the_chart_so_copy_and_fill_down_act_on_cells, pasting_a_copied_chart_that_was_deleted_pastes_the_text_instead
  • tests::duplicate_object_copies_with_a_new_id_beside_the_original (engine: chart and picture, offsets, unique ids, undo, bad id/kind)
  • cargo test -p gridcraft-ui-egui --test clipboard --test interaction --test keyboard_shortcuts, cargo test -p gridcraft-engine --lib every_command_survives
  • cargo clippy -p gridcraft-engine -p gridcraft-ui-egui --all-targets -- -D warnings, cargo fmt --all -- --check
  • docs/ui-parity.md updated (keyboard row and revision history)
  • (macOS, Apple silicon)

@FredApps FredApps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReviewerA — source-read COMMENT (not an approval)

I read the head of the diff: the selected chart, picture or shape can be copied, pasted and duplicated. object.duplicate takes {kind, id, dx?, dy?}, offsets the copy by 12 points right and down so it shows beside the original, puts it on top of the other objects, gives it a new id and returns {kind, id}. Excel's Ctrl+D on an object duplicates it with an offset, and Ctrl+C/V pastes with an offset, so the oracle matches. A copied object that refers to cells (a chart's data range) keeps its references; pasting onto another sheet must keep the references to the original sheet. The copy should be one undo step. I have not run it, and no exact-head CI result exists for this head. Nothing was run.

I ran nothing and have no exact-head CI result for this PR, so this is not an approval. Where a diff was long I read its code hunks rather than every generated or documentation line.

Status: waiting_author.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cant copy and paste graphs ie create duplicates

2 participants