Skip to content
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

Pasting table into a selected table fragment (bigger then one cell) #6120

Closed
jodator opened this issue Jan 22, 2020 · 6 comments · Fixed by #6786
Closed

Pasting table into a selected table fragment (bigger then one cell) #6120

jodator opened this issue Jan 22, 2020 · 6 comments · Fixed by #6786
Assignees
Labels
bc:minor Resolving this issue will introduce a minor breaking change. package:table type:feature This issue reports a feature request (an idea for a new functionality or a missing option).

Comments

@jodator
Copy link
Contributor

jodator commented Jan 22, 2020

📝 Provide a description of the new feature

Requires only #6113 to be finished beforehand.

When pasting a table into a selected table fragment three scenarios might occur:

  1. Selected table content area matches pasted table
  2. Selected table content is smaller than the pasted table
  3. Selected table content is bigger than the pasted table cell

Scenario 1: Current table content is cleared (potentially linked to #6119) and replaced by the content of a pasted table.
Scenario 2: Only the part of the pasted table that matches the selection will be pasted into the editor.
Scenario 3: Pasted table data will be replicated in the column by column, then row by row fashion.

Scenario 1 should be addressed first. Scenario 2 & 3 might be moved to a follow-up. Scenario 2 & 3 should be then blocked.


If you'd like to see this feature implemented, add a 👍 reaction to this post.

@jodator jodator added type:feature This issue reports a feature request (an idea for a new functionality or a missing option). package:table labels Jan 22, 2020
@Reinmar Reinmar added this to the nice-to-have milestone Jan 27, 2020
@jodator jodator self-assigned this Mar 31, 2020
@jodator jodator modified the milestones: nice-to-have, iteration 32 Apr 30, 2020
@jodator
Copy link
Contributor Author

jodator commented May 6, 2020

Should we alter model.insertContent() behavior when pasting or introduce an event to prevent default Clipboard's behavior?

Background

In this task we need to override default clipboard handling if:

  • (A) there's a selection over one or many table cells or inside a table cell
  • (B) there's a table pasted

This can be done on multiple levels:

  1. listening to view.document events - paste
  2. listening to Clipboard plugin event inputTransformation
  3. listening to model decorated method event insertContent

I think that a best scenario would be to operate on model only here to check the selection (A) and checking what is going to be inserted from a model's POV (B) after the conversion.

In the above scenarios 1 & 2 are happening before conversion - so we operate on a view structure. I don't think this is good way to convert the same content multiple times as Clipboard will already convert the inserted content.

The scenario 3 is IMO a bit too late and too broad (we would override this behavior for every case - not only a paste but all other model.insertContent in above cases. Which might actually by OK.

Proposals

  1. The model.insertConent() is OK as it will handle "all" cases.
  2. We should introduce new event for Clipboard plugin that would allow to override model manipulation.

@Reinmar
Copy link
Member

Reinmar commented May 6, 2020

Fun fact – insertContent() is observable and works on that level of abstraction exactly because of tables :D I had exactly tables in mind when forcing exposing this util 4 years ago.

I now keep my fingers crossed for it to work in this case 🤣

@jodator
Copy link
Contributor Author

jodator commented May 6, 2020

I now keep my fingers crossed for it to work in this case rofl

It does work in a simplest scenario so far :crossed_fingers: 

@jodator
Copy link
Contributor Author

jodator commented May 7, 2020

Edge cases (possible follow ups):

  1. Selected table fragment is not rectangular (ie you could not merge it using MergeCellsCommand)
  2. Selected table fragment is rectangular but last row is rowspanned by previous (able to merge using MergeCellsCommand but current utils doesn't allow that.
  3. Similar case as 2. but with columns / colspans.

Cases 2 & 3 looks like easy to handle as only different check is required.

Case 1 requries cropping selection from table - might be harder.

@jodator
Copy link
Contributor Author

jodator commented May 8, 2020

Document MINOR BREAKING CHANGE: removed cropTable() function.

@jodator
Copy link
Contributor Author

jodator commented May 8, 2020

Scenario 3: Pasted table data will be replicated in the column by column, then row by row fashion.

Moved to a follow-up: #6769.

@jodator jodator added the bc:minor Resolving this issue will introduce a minor breaking change. label May 11, 2020
niegowski added a commit that referenced this issue May 15, 2020
Feature (table): Introduce pasting tables into a selected table fragment. Closes #6120.

MINOR BREAKING CHANGE: The `cropTable()` utility method was removed. Use `cropTableToDimensions()` instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bc:minor Resolving this issue will introduce a minor breaking change. package:table type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants