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

fix: do not trigger data communicator reset when no filters #6712

Merged
merged 4 commits into from
Oct 11, 2024

Conversation

vursen
Copy link
Contributor

@vursen vursen commented Oct 11, 2024

The setDropMode and setRowsDraggable methods were updated to call dataCommunicator.reset to ensure drop filters are applied when the drop mode is set dynamically. While this fixed the original issue, it introduced an unintended side effect: an annoying scroll position reset that now happens e.g. when setting the drop mode dynamically on drag start.

Unfortunately, resolving this new issue completely is impossible without introducing new API in DataCommunicator. An API that would allow us to request refreshItem for only items that are in the viewport.

Since not working filters appear to be a more critical issue, it was decided to keep the original change but try to mitigate its impact by preventing the data reset when no filters are set.

Fixes #6662

@vursen vursen force-pushed the do-not-trigger-data-reset-without-filters branch from 910e946 to 04dc622 Compare October 11, 2024 10:46
Copy link

sonarcloud bot commented Oct 11, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@vursen
Copy link
Contributor Author

vursen commented Oct 11, 2024

Meanwhile, I created #6713 in which I described a solution that could help resolve this issue completely.

@vaadin-bot
Copy link
Collaborator

Hi @vursen and @vursen, when i performed cherry-pick to this commit to 23.5, i have encountered the following issue. Can you take a look and pick it manually?
Error Message:
Error: Command failed: git cherry-pick 45ff63f
error: could not apply 45ff63f... fix: do not trigger data communicator reset when no filters (#6712)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

vursen added a commit that referenced this pull request Oct 11, 2024
vursen added a commit that referenced this pull request Oct 11, 2024
@gektron
Copy link

gektron commented Oct 12, 2024

Thank you so much for implementing a fix. What would be the release schedule for this? Will it make 24.5?

@vursen
Copy link
Contributor Author

vursen commented Oct 12, 2024

@gektron Yes, this fix will be included in the upcoming 24.5 release.

@vaadin-bot
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TreeGrid drag and drop causes complete data refresh/reset each time drag is started
4 participants