Skip to content

feat(ui5-tabcontainer, ui5-list): add events for reordering items by mouse #8265

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

Merged
merged 51 commits into from
Mar 14, 2024

Conversation

dimovpetar
Copy link
Contributor

@dimovpetar dimovpetar commented Feb 9, 2024

Properties and events will be documented with another PR.

BGSOFUIRODOPI-3189

@dimovpetar dimovpetar force-pushed the drag_and_drop branch 2 times, most recently from 0ab7b15 to 09cdb06 Compare February 19, 2024 13:13
@dimovpetar
Copy link
Contributor Author

TODO: Rename DropPlacement" to "Placement"

Done

@dimovpetar
Copy link
Contributor Author

If in "ui5-move-over" the "e.preventDefault();" is not called, the needle shouldn't be shown.

Fixed

@LidiyaGeorgieva
Copy link
Contributor

LidiyaGeorgieva commented Mar 11, 2024

I tested noticed the following:
For the list (http://localhost:8080/packages/main/test/pages/ListDragAndDrop.html)

  1. If I drag "Spain" over the "Germany" and then drag "Spain" on the level of "Bulgaria" (revert nesting) the "Germany" is empty (as expected), but stays grey:
    image

For the tab-container (http://localhost:8080/packages/main/test/pages/TabContainerDragAndDrop.html): 2. If I open the right overflow and try to move "Twenty Four" above "Twenty Three" I see the correct cursor (the hand) and the element becomes grey, but when I'm out of the elements boundaries it doesn't move image

  1. (Maybe related with the previous one?) When I drag "Fourteen" over "Thirteen" and nest it it works as expected. But if I try to move "Fourteen" back to its previous place I can't drag it (as described before - looks like it can be dragged, but it doesn't move):
    image
    [update] I noticed that in this case if I start dragging over the padding (not the text) it works, but it is still strange user experience to not work if the element is grabbed in the center (over the text)
  2. In tab-container .focus() after drop not always work well
  3. There are differences with the design

5.1. The background of the dragged tab is transparent: design: image implementation: image
5.2. The circle of the needle is cut design: image implementation: image
5.3. When I start dragging element the colors and the focus is not as specified design: image implementation: image

  1. Fixed
  2. Issue that you can' drag the list items by their text, which in this case is slotted element - seems like bug or feature in Chromium based browsers. See https://issues.chromium.org/issues/328263459. I will wait to see if they will fix it, as there is no workaround I can think of at the moment
  3. Same as 2.
  4. Will be discussed (TODO)
    1. fixed
    2. fixed
    3. fixed
  1. Text color is OK now, but the background of "Germany" stays grey (as it is hovered):
image
  1. There are light grey vertical borders visible (for Horizon Light/Dark and Fiori 3 Light/Dark themes):
image

@dimovpetar
Copy link
Contributor Author

I tested noticed the following:
For the list (http://localhost:8080/packages/main/test/pages/ListDragAndDrop.html)

  1. If I drag "Spain" over the "Germany" and then drag "Spain" on the level of "Bulgaria" (revert nesting) the "Germany" is empty (as expected), but stays grey:
    image

For the tab-container (http://localhost:8080/packages/main/test/pages/TabContainerDragAndDrop.html): 2. If I open the right overflow and try to move "Twenty Four" above "Twenty Three" I see the correct cursor (the hand) and the element becomes grey, but when I'm out of the elements boundaries it doesn't move image

  1. (Maybe related with the previous one?) When I drag "Fourteen" over "Thirteen" and nest it it works as expected. But if I try to move "Fourteen" back to its previous place I can't drag it (as described before - looks like it can be dragged, but it doesn't move):
    image
    [update] I noticed that in this case if I start dragging over the padding (not the text) it works, but it is still strange user experience to not work if the element is grabbed in the center (over the text)
  2. In tab-container .focus() after drop not always work well
  3. There are differences with the design

5.1. The background of the dragged tab is transparent: design: image implementation: image
5.2. The circle of the needle is cut design: image implementation: image
5.3. When I start dragging element the colors and the focus is not as specified design: image implementation: image

  1. Fixed
  2. Issue that you can' drag the list items by their text, which in this case is slotted element - seems like bug or feature in Chromium based browsers. See https://issues.chromium.org/issues/328263459. I will wait to see if they will fix it, as there is no workaround I can think of at the moment
  3. Same as 2.
  4. Will be discussed (TODO)
    1. fixed
    2. fixed
    3. fixed
  1. Text color is OK now, but the background of "Germany" stays grey (as it is hovered):
image 5. There are light grey vertical borders visible (for Horizon Light/Dark and Fiori 3 Light/Dark themes): image
  1. Looks like :hover pseud class remains active on the wrong element. Opened Chromium issue https://issues.chromium.org/issues/329096331. Anyways, this is not a real case, it it used only for testing in our internal test page
  2. Fixed

Copy link
Contributor

@LidiyaGeorgieva LidiyaGeorgieva left a comment

Choose a reason for hiding this comment

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

+1
looks good to me

@dimovpetar dimovpetar merged commit c4383ea into main Mar 14, 2024
@dimovpetar dimovpetar deleted the drag_and_drop branch March 14, 2024 15:37
dimovpetar added a commit that referenced this pull request Mar 14, 2024
dimovpetar added a commit that referenced this pull request Mar 19, 2024
nnaydenow pushed a commit that referenced this pull request Mar 20, 2024
nnaydenow pushed a commit that referenced this pull request Mar 20, 2024
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.

7 participants