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

Support mixed content in table paste scenarios #6817

Closed
jodator opened this issue May 14, 2020 · 9 comments
Closed

Support mixed content in table paste scenarios #6817

jodator opened this issue May 14, 2020 · 9 comments
Labels
package:table resolution:expired This issue was closed due to lack of feedback. status:discussion status:stale type:improvement This issue reports a possible enhancement of an existing feature.

Comments

@jodator
Copy link
Contributor

jodator commented May 14, 2020

During work on #6120 we've decided that it would be better to fallback to the old behavior when pasting table with other content. In other word pasting is allowed only when table is the only content being pasted over a selected table fragment.


If you'd like to see this issue resolved please react with 👍 to this message.

@jodator jodator added type:improvement This issue reports a possible enhancement of an existing feature. package:table squad:table labels May 14, 2020
@Reinmar Reinmar added this to the nice-to-have milestone May 18, 2020
@jodator
Copy link
Contributor Author

jodator commented Jun 2, 2020

Let's start this with some research

  • what to other do?
  • what to do with empty content around table (remove?)
  • what to do when non-empty content around table
  • 2+ tables?

If task could grow too much let's create follow-ups for non-trival cases.

@jodator jodator modified the milestones: nice-to-have, iteration 33 Jun 2, 2020
@niegowski niegowski self-assigned this Jun 3, 2020
@niegowski
Copy link
Contributor

Word

Cases: text + table + text, br + table + br, p + table + p (empty)

  1. Paste to single cell - text + nested table + text (inside table cell).
  2. Paste to multiple cells - first text in one cell, rest in the cell below (nested table), then repeats pattern. When only one row is selected then pasted are only empty elements and it looks like nothing happened.

GDocs

Case: text + table + text, p + table + p (empty)

  1. Paste to single cell - text + nested table + text (inside table cell).
  2. Paste to multiple cells - repeats pattern in every cell as for single cell.

Case: br + table

Behaves the same way as only table would be in the clipboard.

Cases: table + br, br + table + br

Merges br into last table cell and then behaves as above (this is odd behavior).

@niegowski
Copy link
Contributor

Also I noticed:

  • when table is copied from web page, Word or GDocs and selection was starting just above table and ending just below table it does'n put there <p> element but <br>.
  • sometimes when copying table from web page, there were <table> missing (only <tr>-s and <td>-s) and sometimes it was just list of <td>-s.

@niegowski
Copy link
Contributor

niegowski commented Jun 3, 2020

My proposal (since we don't support nested tables):

  • If before/after a table there is no content (just empty <p> or <br>) then we should extract only table and use it in our currently handled scenarios as it would be the only clipboard content.
  • If there are multiple block elements in the clipboard (text + table + text) we could treat it as a table extended above and below with block elements that was around table. 
    Example:
This paragraphs and tableWould be pasted as this
Zrzut ekranu 2020-06-3 o 17 58 39

This would be partly replicating odd behavior of Word. I think this would be very odd and undesired but at least it would show to the user that there was some reaction and user can "undo" and pick better clipboard content for pasting. WDYT @jodator @oleq

@niegowski niegowski reopened this Jun 3, 2020
@jodator
Copy link
Contributor Author

jodator commented Jun 4, 2020

Case 1: I think that this is a no-brainer here - this would save some PITAs when selection was not ideal. We could do that.

Case 2: Could you also check non-text content (at least images) and lists? Probably a behavior would be similar but just to confirm.

text (inside table cell)

Could you also attach screenshot? I'm not sure in which table (outer or nested) your writing here. But judging from the proposal it is in nested (so the text/p is wraped by a table cell and moved to a pasted table).

Another thing to consider is just wrapping everything before/after table in one <td> - might be easier when mixed content (like <p> + <img> + <ul> + <table> ) is pasted. (I'm thinking here to paste it as <table><td><p><img><ul></td>...<td></td></table>.

@oleq
Copy link
Member

oleq commented Jun 4, 2020

I agree that "Case 1" is pretty obvious and you conclusions make sense 👍 .


As for the "Case 2", I'm not too thrilled about it. You proposed a solution that does everything to squeeze the pasted content into the editor but in 90% of cases, users will not expect/want that. It does preserve the content in some sort of way but this is weird.

Simply: this case asks for nested tables support and I think the time you will spend hacking and wrapping and who knows what, for instance:

Another thing to consider is just wrapping everything before/after table in one <td> - might be easier when mixed content (like <p> + <img> + <ul> + <table> ) is pasted.

will be way more productive if it went towards nested tables support, which would not only resolve the "Case 2" but bring a new, real value to the editor as a product.

BTW: as far as I read the issue, the feature is supported, it has to be enabled and only minor issues must be addressed (@jodator?). Maybe we can research this first before we dig into something weird?

@jodator
Copy link
Contributor Author

jodator commented Jun 4, 2020

will be way more productive if it went towards nested tables support, which would not only resolve the "Case 2" but bring a new, real value to the editor as a product.

BTW: as far as I read the issue, the feature is supported, it has to be enabled and only minor issues must be addressed (@jodator?). Maybe we can research this first before we dig into something weird?

The table in table is not researched at all ATM and I have constant fear that it might not be an easy task. We might split that to handle only case 1 for now.

As @niegowski said on F2F - maybe pasting everything else for now would be OK for scenario 2. I don't have a strong feeling for either way.

I'd go with a follow-up for Case 1 and leave this open.

@jodator jodator modified the milestones: iteration 33, nice-to-have Jun 4, 2020
@jodator jodator added squad:dx squad:core Issue to be handled by the Core team. labels Jul 28, 2020
@Reinmar Reinmar removed squad:core Issue to be handled by the Core team. squad:table labels Jul 28, 2020
@pomek pomek removed this from the nice-to-have milestone Feb 21, 2022
@CKEditorBot
Copy link
Collaborator

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may still be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

@CKEditorBot
Copy link
Collaborator

We've closed your issue due to inactivity over the last year. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).

@CKEditorBot CKEditorBot added the resolution:expired This issue was closed due to lack of feedback. label Nov 12, 2023
@CKEditorBot CKEditorBot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:table resolution:expired This issue was closed due to lack of feedback. status:discussion status:stale type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

No branches or pull requests

6 participants