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

[WIP] Enable table cell merge and unmerge #17261

Draft
wants to merge 3 commits into
base: trunk
Choose a base branch
from

Conversation

adamsilverstein
Copy link
Member

@adamsilverstein adamsilverstein commented Aug 29, 2019

Closes #15821

Description

The following features encompass table cell merging and unmerging.

  • Click drag to select a range.
  • Click then shift-click to select a range.
  • Merge cells menu: merges cells by adding colspan/rowspan to the root cell, and removing remaining sell, combing their content into the first cell.
  • Merging should trigger undo level and dirty post.
  • Merged areas should be tracked or calculable
  • When saving, colspan and rowspan should be included.
  • "Unmerge" command should be available when selection includes any merged area.
  • Unmerging re-divides and selected merged areas. content remains in the root cell

How has this been tested?

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@talldan
Copy link
Contributor

talldan commented Aug 30, 2019

Hey @adamsilverstein, it's great that you're taking a look at this.

I have a couple of PRs lined up for the table block that have some crossover, and it'd be great to think about how they could work together. See:

One of the things I was heading towards is having a single type of selection object (#16493 implements this, but needs some tidy-up):

selection: {
  type: 'cell'|'column'|'row'|'table',
  // ... selectionData
};

I think this could be applied in your code, the selectionData for a 'range' could look like this:

selection: {
  type: 'range',
  cells: [],

Or

selection: {
  type: 'range',
  start: { // ... },
  end: { // ... }
}

#16493 also implements CSS rules for showing borders around multi-cell selections which might be useful for this PR.

@mapk
Copy link
Contributor

mapk commented Sep 21, 2020

Looks like this has gotten stale. Any chance it can be picked up again, @talldan @adamsilverstein ?

@adamsilverstein
Copy link
Member Author

I would love to @mapk, thanks for the ping! I'll put this on my list.

@adamsilverstein
Copy link
Member Author

👋 Hey @talldan - I'm picking this back up and think I should probably start with the styles and structures you laid out in #16493 - are you hoping to revisit that PR any time soon?

@talldan
Copy link
Contributor

talldan commented Nov 24, 2020

@adamsilverstein I probably won't be able to revisit #16493 any time soon.

With the table block, I'm wondering whether the time is right to start focusing more on looking at solutions that involve inner blocks, like described in #18768. What do you think?

My concern is that if we add more features like cell merging, the transition to using inner blocks becomes more difficult as those features then have to be adapted to also work with inner blocks.

@strarsis
Copy link
Contributor

strarsis commented Feb 2, 2021

So this PR would first transition to using inner blocks as table cells and then (re)implement the merging?

Base automatically changed from master to trunk March 1, 2021 15:42
@jeffpaul
Copy link
Member

@adamsilverstein any thoughts on switching up the approach on this to use inner blocks as @talldan referenced is a potential option discussed in #18768?

@adamsilverstein
Copy link
Member Author

@adamsilverstein any thoughts on switching up the approach on this to use inner blocks as @talldan referenced is a potential option discussed in #18768?

@jeffpaul Probably makes sense although I haven't looked at the way inner blocks work carefully enough to be sure. My effort here stalled and I have been focused elsewhere, so I welcome any contributions/help./

@jdaviescoates
Copy link

I'd love to see this actually merged. What's the current status/ plan? Thanks!

@ajotka
Copy link
Contributor

ajotka commented Jan 10, 2022

@adamsilverstein I'm working on your proposal by my own (I don't want to promise anything yet!), but I found a bug.
When you add header and/or footer, selecting don't work. If you select cell with index [0 0] it will also select header and footer with index [0 0].
I'm just reporting for keeping it in mind.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Table Affects the Table Block [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Table block: Merge/Unmerge cells
7 participants