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

Select all columns in previous table rows when the last row is fully selected. #17697

Merged
merged 4 commits into from
Feb 25, 2025

Conversation

Mati365
Copy link
Member

@Mati365 Mati365 commented Dec 30, 2024

Suggested merge commit message (convention)

Fix (table): Improve selection of the cells with colspan attribute. Closes #17538

MINOR BREAKING CHANGE (table): The TableSelection#getSelectedTableCells method now considers cells with a colspan. If the selection ends on a cell with a colspan, it will be adjusted to the end of that cell, and all cells above it will be returned, instead of just one.

MINOR BREAKING CHANGE (table): The TableSelection#setCellSelection method has been updated to behave consistently with the updated getSelectedTableCells, ensuring that cell selection is aligned with the new behavior regarding cells with a colspan.


Additional information

I normalized cell-span selection behaviour to make it work in similar way it works in TinyMCE.

Before

before-fix-selection-v3-2024-12-30_13.21.34.mp4
before-selection-fix-2024-12-30_10.40.55.mp4

After

after-fix-selection-v3-2024-12-30_13.19.51.mp4
after-fix-selection-2024-12-30_13.17.30.mp4

@Mati365 Mati365 force-pushed the ck/17538 branch 2 times, most recently from 20d2fd4 to 4e43749 Compare December 30, 2024 09:39
@Mati365 Mati365 requested review from niegowski and arkflpc December 30, 2024 10:24
@Mati365
Copy link
Member Author

Mati365 commented Dec 30, 2024

GDocs behaviour:

gdocs-2024-12-30_12.03.54.mp4

@charlttsie
Copy link
Contributor

I've done some testing, and the fix looks good to me. The issue no longer reproduces, and I haven't found any regressions 👍

gorzelinski
gorzelinski previously approved these changes Jan 20, 2025
@arkflpc
Copy link
Contributor

arkflpc commented Feb 25, 2025

I have one doubt about the suggested merge commit message. You mentioned getSelectedTableCells but it seems not be affected by the changes (setCellSelection is). You also didn't mention what class this method belongs to.

arkflpc
arkflpc previously approved these changes Feb 25, 2025
@Mati365 Mati365 dismissed stale reviews from arkflpc and gorzelinski via 17c26e7 February 25, 2025 12:04
@Mati365 Mati365 changed the base branch from master to master-it84 February 25, 2025 12:04
@Mati365 Mati365 merged commit 2a8166e into master-it84 Feb 25, 2025
9 checks passed
@Mati365 Mati365 deleted the ck/17538 branch February 25, 2025 13:18
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.

For specific tables, properties are not applied to some cells
4 participants