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

XWIKI-21009: LiveData alternative to drag controls #2809

Merged
merged 4 commits into from
Jan 19, 2024
Merged

XWIKI-21009: LiveData alternative to drag controls #2809

merged 4 commits into from
Jan 19, 2024

Conversation

Sereza7
Copy link
Contributor

@Sereza7 Sereza7 commented Jan 19, 2024

Jira URL

https://jira.xwiki.org/browse/XWIKI-21009

Changes

Description

  • Fixed the lost pointer cursor on the sort caret
  • Fixed broken translations
  • Fixed the unexpected change in behavior for the resize handle.
  • Improved cursor on various elements to fit the features they should help use.
  • Made the resize handles invisible when not focused again.

Clarifications

  • The position of the resize-handle is different, which created an incompatibility with the way the mousedownmove event was taken care of in directives.js
  • Some translations were not added to Logic.js and therefore didn't make it into the vue template as expected.
  • In order to minimize the UI changes that didn't go through the forum for committer feedback on UI, some style for the resize-handle has been reverted to be closer to what it used to be.

Screenshots & Video

Demo video of the livedata after this PR

Executed Tests

No specific tests, built successfully mvn clean install -f xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-webjar

Expected merging strategy

@Sereza7
XWIKI-21009: LiveData alternative to drag controls
94d04f9 
* Fixed the lost pointer cursor on the `sort` caret
* Fixed broken translations
@Sereza7
XWIKI-21009: LiveData alternative to drag controls
fb525ea 
* Fixed the unexpected change in behavior for the resize handle.
* Improved cursor on various elements to fit the features they should help use.
@Sereza7
XWIKI-21009: LiveData alternative to drag controls
51f8cce 
* Made the resize handles invisible when not focused again.
@manuelleduc
Copy link
Contributor

Regarding the test command, I often user the following pattern:

mvn clean install -amd -f xwiki-platform-core/xwiki-platform-livedata -pl :xwiki-platform-livedata-webjar  -Pquality,integration-tests,docker
  • -pl list the modules with direct changes
  • -amd also build the modules that depend on the modules listed in -pl
  • -f limit the scope to a sub-module (otherwise, -amd can lead to a very large list of modules)
  • -Pquality,integration-tests,docker to also have the impacted test-docker modules executed, and quality checked

@manuelleduc
Copy link
Contributor

Looks good, last remark. When a resize handle becomes visible, it's impacting the size of the column, leading to a small twitch.

manuelleduc
manuelleduc reviewed Jan 19, 2024
View reviewed changes
...tform-core/xwiki-platform-livedata/xwiki-platform-livedata-webjar/src/main/vue/directives.js Outdated
@@ -61,7 +60,8 @@ export const mousedownmove = {
// Bind event listeners
window.addEventListener("mousemove", mousemoveHandler);
window.addEventListener("mouseup", removeMousemoveHandler);
window.addEventListener("focusout", removeMousemoveHandler);
// Stop propagation of the mousedown event
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe explain why the propagation stop is required (and what are the consequences otherwise).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked it again, and with the move I just made, it's not needed anymore.

@Sereza7
XWIKI-21009: LiveData alternative to drag controls
fdee552 
* Moved the resize handle back to where it waws initially
* Updated style to fit this new position
* Removed a useless stoppropagation on an event
@Sereza7
Copy link
Contributor Author

Sereza7 commented Jan 19, 2024 •
edited
Loading

@manuelleduc
#2809 (comment)

Thanks for this pattern! Keeping it in a safe place to use it on other PRs later on :)
Is that okay if I share it somewhere on the dev wiki to make it easier for new contributors?

Looks good, last remark. When a resize handle becomes visible, it's impacting the size of the column, leading to a small twitch.

Seems like was created by the changed position of the resize handle. I wanted to remove the absolute positioning of this handle by moving it, but it creates complications (also part of the reason why it was misplaced when dragging the rightmost column resizer to the left...)
So I decided to revert those changes in fdee552

Here is a new demo of the changes.

Executed tests

Successfully passed mvn clean install -amd -f xwiki-platform-core/xwiki-platform-livedata -pl :xwiki-platform-livedata-webjar -Pquality,integration-tests,docker 👍

@manuelleduc
Copy link
Contributor

Is that okay if I share it somewhere on the dev wiki to make it easier for new contributors?

Sure :)

@manuelleduc manuelleduc merged commit 3f95f97 into xwiki:master Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@manuelleduc manuelleduc manuelleduc left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Sereza7 @manuelleduc