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

Allow action handler to abort multi-select, resetting last index state #918

Merged
merged 8 commits into from
Dec 7, 2021

Conversation

ahmacleod
Copy link
Contributor

@ahmacleod ahmacleod commented Dec 2, 2021

Description

Adds ability to "abort" a selection, resetting the internally-tracked last selected row index. This allows multi-select to function as expected after rejecting a user's selection.

Use case

Say we want to prevent the user from selecting more than 10 rows.

If the user clicks row 1 and then shift-clicks row 11, we can inspect the proposed new selection in the onSelect action, determine it is too large, and show an error without making a change.

However, if they subsequently shift-click row 5, Ember Table will attempt to add rows 5-11 to the selection, as row 11 was the last clicked.

By calling abort(), the internal state is reverted, and shift-clicking row 5 will select rows 1-5.

Docs Update

  • Breaks out examples into separate files to fix broken markdown formatting.

image
image

@ahmacleod ahmacleod marked this pull request as ready for review December 3, 2021 16:25
Copy link
Contributor

@twokul twokul left a comment

Choose a reason for hiding this comment

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

The approach looks good to me! :shipit:


There is, however, some internal state that needs to be reset to fully abort a user selection. For example, Ember Table tracks the _last selected_ row in order to determine the range of rows affected in a user multi-selection. If the intent is to completely prevent a user selection, this value must not change when the action is aborted. Otherwise, a subsequent user multi-selection may target the wrong rows.

To reset all internal state relating to an attempted user selection, call the `abort` function in the options object passed to the `onChange` action handler:
Copy link
Contributor

Choose a reason for hiding this comment

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

why cannot we automatically perform the cleanup ? 🤔 ( i believe there are reasons, but i'm just not able to figure out from the code )

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 think from Ember Table's perspective, there's no reason to perform any cleanup at all. Selection is purely DDAU, so ET can't infer very much when it receives a new selection attr. The notion of a forbidden or cancelled selection is something we've made up in the application layer.

My intent with this PR was to provide what's needed to implement this logic in the application, but not force it on anyone, or change how existing applications use the onSelect action. I've kept the abort handler rather vague in its description so that we can tweak exactly what it does in the future.

@ahmacleod ahmacleod merged commit d35d2af into master Dec 7, 2021
@ahmacleod ahmacleod deleted the alex.macleod.temp/abort-multi-select branch December 7, 2021 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants