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.

Sign up for GitHub

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

Jump to bottom

fix: handle focus button mode columns for reordering #7273

Merged
merged 10 commits into from
Mar 28, 2024
Merged

fix: handle focus button mode columns for reordering #7273

merged 10 commits into from
Mar 28, 2024

Conversation

ugur-vaadin
Copy link
Contributor

@ugur-vaadin ugur-vaadin commented Mar 26, 2024 •
edited
Loading

Description

This is not to a GridPro edit column issue, but can also be reproduced using regular columns with _focusButtonMode property. When focus button mode is used, the cell is content is wrapped into a div. Because of this, the cell from point is not correctly determined during tracking events.

This PR adds another clause for extracting the cell from point, which enables reordering focus button mode columns.

Fixes #7229

Type of change

  • Bugfix
  • Feature

Checklist

  • I have read the contribution guide: https://vaadin.com/docs/latest/contributing/overview
  • I have added a description following the guideline.
  • The issue is created in the corresponding repository and I have referenced it.
  • I have added tests to ensure my change is effective and works as intended.
  • New and existing tests are passing locally with my change.
  • I have performed self-review and corrected misspellings.

@ugur-vaadin ugur-vaadin mentioned this pull request Mar 26, 2024
Merged
8 tasks
@ugur-vaadin ugur-vaadin marked this pull request as ready for review March 27, 2024 09:14
@vursen
Copy link
Contributor

vursen commented Mar 27, 2024 •
edited
Loading

The original reordering issue appears to have been fixed by #7274. The fact that header cells had focus button divs was a regression. Do we still need this PR? I guess the change that unskips the test is still needed.

@ugur-vaadin
Copy link
Contributor Author

The original reordering issue appears to have been fixed by #7274. The fact that header cells had focus button divs was a regression. Do we still need this PR? The test can be uncommented though.

I would say it is still relevant because it still fixes the reordering issue when the drop is on the body cells.

@vursen
Copy link
Contributor

vursen commented Mar 27, 2024

I would say it is still relevant because it still fixes the reordering issue when the drop is on the body cells.

Good point, thanks.

@ugur-vaadin ugur-vaadin force-pushed the fix-handle-focus-button-mode-columns-for-reordering branch from 03c8855 to 4dde741 Compare March 27, 2024 13:49
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@ugur-vaadin ugur-vaadin merged commit 27a5d55 into main Mar 28, 2024
9 checks passed
@ugur-vaadin ugur-vaadin deleted the fix-handle-focus-button-mode-columns-for-reordering branch March 28, 2024 07:35
vaadin-bot pushed a commit that referenced this pull request Mar 28, 2024
@ugur-vaadin @vursen @vaadin-bot
fix: handle focus button mode columns for reordering (#7273)
410ddc2 
* fix: handle focus button mode columns for reordering

* Update packages/grid/src/vaadin-grid-column-reordering-mixin.js

Co-authored-by: Sergey Vinogradov <[email protected]>

* chore: fix format

* test: revert skipped tests that pass with the fix

* test: add test for focus button mode column reordering

* test: add test for reordering by dropping column on body cells

* test: add test for focus button mode column reordering

* test: remove duplicate test

* Update packages/grid/src/vaadin-grid-column-reordering-mixin.js

Co-authored-by: Sergey Vinogradov <[email protected]>

* Update packages/grid/test/column-reordering.common.js

Co-authored-by: Sergey Vinogradov <[email protected]>

---------

Co-authored-by: Sergey Vinogradov <[email protected]>
web-padawan pushed a commit that referenced this pull request Mar 28, 2024
@vaadin-bot
fix: handle focus button mode columns for reordering (#7273) (#7282)
42df54a
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.4.0.alpha20 and is also targeting the upcoming stable 24.4.0 version.

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

@vursen vursen vursen approved these changes

Assignees
No one assigned
Labels
cherry-picked-24.3 prerelease version for Vaadin 24.4.0 Released with Vaadin 24.4.0.alpha20 target/24.3
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

GridPro column drag&drop reordering does not work with GridPro edit columns
3 participants
@ugur-vaadin @vursen @vaadin-bot