Skip to content

fix(sort-handle): allow move and reorder events to be cancelled#12095

Merged
driskull merged 5 commits intodevfrom
dris0000/sort-handle-event-fix
May 12, 2025
Merged

fix(sort-handle): allow move and reorder events to be cancelled#12095
driskull merged 5 commits intodevfrom
dris0000/sort-handle-event-fix

Conversation

@driskull
Copy link
Copy Markdown
Member

@driskull driskull commented May 5, 2025

Related Issue: #11774

Summary

  • allows events to be cancelled
  • This is inconsistent with the other events where the propagation is being stopped when needed
    • see
      private handleSortHandleBeforeOpen(event: CustomEvent<void>): void {
      event.stopPropagation();
      this.calciteBlockSortHandleBeforeOpen.emit();
      }
      private handleSortHandleBeforeClose(event: CustomEvent<void>): void {
      event.stopPropagation();
      this.calciteBlockSortHandleBeforeClose.emit();
      }
      private handleSortHandleClose(event: CustomEvent<void>): void {
      event.stopPropagation();
      this.sortHandleOpen = false;
      this.calciteBlockSortHandleClose.emit();
      }
      private handleSortHandleOpen(event: CustomEvent<void>): void {
      event.stopPropagation();
      this.sortHandleOpen = true;
      this.calciteBlockSortHandleOpen.emit();
      }

@driskull driskull requested a review from jcfranco May 5, 2025 16:36
@driskull driskull marked this pull request as ready for review May 5, 2025 16:36
@driskull driskull added the skip visual snapshots Pull requests that do not need visual regression testing. label May 5, 2025
@github-actions github-actions Bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label May 5, 2025
@driskull
Copy link
Copy Markdown
Member Author

driskull commented May 5, 2025

@jcfranco this fixes the issue. However, I noticed other sort-handle events are being stopped rather than being cancelled so this is inconsistent.

@jcfranco
Copy link
Copy Markdown
Member

However, I noticed other sort-handle events are being stopped rather than being cancelled so this is inconsistent.

The example from the description is fine since its handling internal events.

If you mean how other public handle events are non-cancellable, we have to flesh out our cancelable event story first. We've been mostly making events cancelable as needed.

Copy link
Copy Markdown
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

Code changes LGTM! Can you add a test before merging?

@@ -136,13 +136,13 @@ export class SortHandle extends LitElement implements InteractiveComponent {
calciteSortHandleClose = createEvent({ cancelable: false });

/** Fires when a move item has been selected. */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Side note: we should add a doc snippet for cancelable events. cc @geospatialem @DitwanP

@driskull driskull merged commit a23105d into dev May 12, 2025
10 checks passed
@driskull driskull deleted the dris0000/sort-handle-event-fix branch May 12, 2025 18:03
benelan pushed a commit that referenced this pull request May 14, 2025
benelan added a commit that referenced this pull request May 15, 2025
* origin/dev: (277 commits)
  docs(tokens): consistency pass for new component descriptions (#12148)
  build(preset): use valid TS module resolution (#12151)
  docs: update list of contributors (#12134)
  chore: drop obsolete transforms (#12136)
  chore: release main (#11890) (#12147)
  build(deps): bump @arcgis/lumina, typescript, vite, vitest (#12137)
  chore(preset): fix JSON import in build config (#12142)
  docs(panel, action): update `text-color-pressed` token descriptions (#12140)
  chore: release next
  fix(input-time-picker): invert text color on Windows when each masked input is focused (#12130)
  chore: release next
  refactor(sematic-tokens): update `--calcite-corner-radius-default` to reference correct token (#12131)
  feat(semantic-tokens): add `--calcite-color-text-highlight` tokens (#12068)
  chore: release next
  feat(combobox): add `selectAll` toggle property (#11721)
  chore: release next
  fix(sort-handle): allow move and reorder events to be cancelled (#12095)
  chore: release next
  feat: added spike data, heart chart, and progress bar icons (#12127)
  feat(text-area): Add design tokens for corner radius, shadow, footer background color (#12124)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Bug reports for broken functionality. Issues should include a reproduction of the bug. skip visual snapshots Pull requests that do not need visual regression testing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants