Skip to content

Conversation

@gatzjames
Copy link
Contributor

Highlights:

  • Updates the lists on the sync modal to not use selection and use actions instead. This allows esc to propagate and doesn't require us to manage selection state

@gatzjames gatzjames self-assigned this Aug 1, 2024
@gatzjames gatzjames requested a review from a team August 1, 2024 13:19
@CurryYangxx
Copy link
Member

CurryYangxx commented Aug 2, 2024

When is it appropriate to use onSelectionChange and when to use onAction?

@gatzjames gatzjames force-pushed the feature/ins-4244-pressing-esc-on-the-sync-diff-view-modal-doesnt-work-as branch from 89caafc to 1d9c884 Compare August 2, 2024 09:28
@gatzjames
Copy link
Contributor Author

When is it appropriate to use onSelectionChange and when to use onAction?

Selection comes with many features like multiple selection, select-deselect (allowing empty selection or not), checkbox support etc. while onAction is a more simple api that only fires when an item is pressed/clicked.

Since we don't use any of the advanced features of selection it's simpler to use action here.
If we'd want to allow multiple selection to e.g. select many items and drag-n-drop them to another list we would need to use the selection api.

Also note that they can be used in combination where you can have an action on press and a selection using a checkbox (with different keyboard shortcuts too)

@gatzjames gatzjames enabled auto-merge (squash) August 2, 2024 09:32
@gatzjames gatzjames merged commit 0ffd007 into Kong:develop Aug 2, 2024
@gatzjames gatzjames deleted the feature/ins-4244-pressing-esc-on-the-sync-diff-view-modal-doesnt-work-as branch August 2, 2024 09:41
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.

2 participants