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

List: Expose Events for dragging between Lists #7046

Closed
Csmith246 opened this issue May 26, 2023 · 10 comments
Closed

List: Expose Events for dragging between Lists #7046

Csmith246 opened this issue May 26, 2023 · 10 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. ArcGIS Instant Apps Issues logged by ArcGIS Instant Apps team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. estimate - 5 A few days of work, definitely requires updates to tests. impact - p2 - want for an upcoming milestone User set priority impact status of p2 - want for an upcoming milestone p - low Issue is non core or affecting less that 10% of people using the library

Comments

@Csmith246
Copy link

Csmith246 commented May 26, 2023

Actual Behavior

For the Calcite List component, when you set dragEnabled to true, and the group value of more than two lists to the same key, and then move items between the lists, none of the current Event's on the List get triggered. This makes it difficult to react to drags between lists.

Expected Behavior

When a List Item gets dragged from one List to a different List, maybe trigger events such as calciteListDragOut and calciteListDragIn on the respective lists. This would broadcast what specifically has occurred within the lists, and allow the wider application to perform any behaviors that are tied to a move between lists.

Reproduction Sample

https://codepen.io/csmith55/pen/YzJMzYg?editors=1001
https://codepen.io/geospatialem/pen/OJaGeVN

Reproduction Steps

Notice in the sample that:

  • Each list has the same group value
  • All events are being tracked on List 1

If you drag any list items from list 2 or 3 into list 1, no events are fired. If you drag any out of List 1, still no events are fired.

Reproduction Version

https://www.npmjs.com/package/@esri/calcite-components/v/1.4.1-next.4
1.5.1

Relevant Info

No response

Regression?

No response

Priority impact

p2 - want for current milestone

Impact

Technically, I don't believe this behavior has ever worked, but we got around that in our application by finding a reference to the internal SortableJS object and adding onAdd and onRemove methods. This fix on the Calcite side (b4bbdf3) caused our work around not to work anymore, so an official solution is now desirable. Thanks!

Esri team

ArcGIS Instant Apps

@Csmith246 Csmith246 added 0 - new New issues that need assignment. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. needs triage Planning workflow - pending design/dev review. labels May 26, 2023
@github-actions github-actions bot added impact - p2 - want for an upcoming milestone User set priority impact status of p2 - want for an upcoming milestone ArcGIS Instant Apps Issues logged by ArcGIS Instant Apps team members. labels May 26, 2023
@Csmith246
Copy link
Author

@driskull Would you have any update on this?

@geospatialem
Copy link
Member

@driskull Would you have any update on this?

@Csmith246 With value-list's deprecation, we're working towards parity and support with list and list-item with dragging from #6554. That being said, the parity is to provide functionality as it is depicted in value-list today, so we could repurpose this issue for list and list-item once dragging is supported.

Will update the request to list and list-item. As for a timeline, will be part of an upcoming triage session for future consideration.

@geospatialem geospatialem changed the title Calcite Value List: Expose Events for dragging between Value Lists List: Expose Events for dragging between Lists Jul 19, 2023
@geospatialem geospatialem added enhancement Issues tied to a new feature or request. p - low Issue is non core or affecting less that 10% of people using the library estimate - 3 A day or two of work, likely requires updates to tests. needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. and removed bug Bug reports for broken functionality. Issues should include a reproduction of the bug. needs triage Planning workflow - pending design/dev review. labels Aug 7, 2023
@geospatialem
Copy link
Member

With the list sorting support in 1.5.1, it looks like the events are exposed with the exception if a new list-item is added to the list. The request should include adding a listener if a new list-item is added, perhaps with the calciteListOrderChange event?

Updated Codepen to list/list-item in 1.5.1: https://codepen.io/geospatialem/pen/OJaGeVN

Recording showing events are registered, unless a list-item is added to the first list:
list-event-listeners

@geospatialem geospatialem added 1 - assigned Issues that are assigned to a sprint and a team member. and removed needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. 0 - new New issues that need assignment. labels Aug 8, 2023
@Csmith246
Copy link
Author

@geospatialem we've got a requirement for supporting accessible keyboard dragging between lists, as well. Could we add that to this issue? Or would it be better to make a new enhancement request for that?

@geospatialem
Copy link
Member

@geospatialem we've got a requirement for supporting accessible keyboard dragging between lists, as well. Could we add that to this issue? Or would it be better to make a new enhancement request for that?

Great question, @Csmith246! For tracking could you create a new enhancement request? It seems like a good enhancement to separate out from the events.

@driskull driskull added estimate - 5 A few days of work, definitely requires updates to tests. and removed estimate - 3 A day or two of work, likely requires updates to tests. labels Aug 24, 2023
@driskull
Copy link
Member

driskull commented Aug 24, 2023

We're using the onSort event from SortableJS and that should happen anytime there's an add or remove.

https:/SortableJS/Sortable

// Called by any change to the list (add / update / remove)
onSort: function (/**Event*/evt) {
  // same properties as onEnd
},

We'll need to investigate why its not emitting correctly. My guess is that its something we're doing to fix VDOM errors.

Bumped this issue to estimate-5. cc @geospatialem

@driskull
Copy link
Member

I think we can mark this one as a bug as well.

@geospatialem geospatialem removed the enhancement Issues tied to a new feature or request. label Aug 25, 2023
@geospatialem geospatialem added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Aug 25, 2023
driskull added a commit that referenced this issue Aug 29, 2023
@geospatialem geospatialem added 2 - in development Issues that are actively being worked on. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels Aug 30, 2023
driskull added a commit that referenced this issue Aug 31, 2023
…n dragging between lists (#7614)

**Related Issue:** #7046

## Summary

TLDR: Basically, we need to pause draggable component
connected/disconnected lifecycle events while a component is being
dragged. Having these lifecycle methods kick off during a drag causes
SortableJS errors.

- SortableComponent
- Refactors logic to prevent any SortableJS component from doing its
lifecycle logic when a component is being dragged
- Previously, it was preventing all SortableJS components Sortable from
being destroyed or created but that was causing issues by not emitting
events when an item was moved from one list to another.
- The nested component check that was previously being used isn't ideal
because two different lists don't have to be nested to drag items
between each other.
- We need all lists to still continue emitting events when necessary, we
just don't want their lifecycle methods to kick off when an item is
being dragged. Otherwise, JS errors are thrown.
- Components
- Updates SortableComponent components to not do any lifecycle callbacks
when an item is being dragged to prevent any JS errors that SortableJS
was throwing.
- This was because in connectedCallback, sortable components were
setting up the sortable instance, connecting the observer, modifying
items, etc. We don't want the component to do this while an item is
being dragged.
  - The same thing as above was happening on disconnectedCallback.
- This fix stops all those errors that occurred while dragging an item
from one list to another.

## Assumptions

It is reasonable to not do any lifecycle events for any draggable
component while a component is being dragged
@driskull driskull added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels Aug 31, 2023
@github-actions github-actions bot assigned geospatialem and unassigned driskull Aug 31, 2023
@github-actions
Copy link
Contributor

Installed and assigned for verification.

@geospatialem geospatialem added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Sep 1, 2023
@geospatialem
Copy link
Member

Verified in 1.7.0

@Csmith246
Copy link
Author

Big thank you from Instant Apps! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - verified Issues that have been released and confirmed resolved. ArcGIS Instant Apps Issues logged by ArcGIS Instant Apps team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. estimate - 5 A few days of work, definitely requires updates to tests. impact - p2 - want for an upcoming milestone User set priority impact status of p2 - want for an upcoming milestone p - low Issue is non core or affecting less that 10% of people using the library
Projects
None yet
Development

No branches or pull requests

3 participants