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

Cancel through SelectionController mess up LocalHistory #31

Closed
iazel opened this issue Aug 22, 2023 · 3 comments
Closed

Cancel through SelectionController mess up LocalHistory #31

iazel opened this issue Aug 22, 2023 · 3 comments
Assignees

Comments

@iazel
Copy link

iazel commented Aug 22, 2023

In my app, I can cancel selection through a button. When the button is pressed, I call selectionController.clear(), which will update the internal state of the Grid component through _onSelectedChange, but this method do not call _updateLocalHistory, hence the LocalHistoryEntry isn't removed and as soon as I pop the route, it will crash due to setState(_selectionManager.clear); on a disposed widget.

The fix is rather easy, but first I'd like to understand what's the need for this LocalHistoryEntry?

By the way, we need to add impliesAppBarDismissal: false to the LocalHistoryEntry, given that it will not dismiss the bar, at least in my case.

@hcbpassos
Copy link
Owner

hey there :)

thanks for the descriptive issue and for spotting the bug.

The fix is rather easy, but first I'd like to understand what's the need for this LocalHistoryEntry?

as you can check in this test, by adding a new entry to the local history, the widget unselects all items whenever we try to pop the route. this behavior is what causes the selection to be automagically cleared when the user taps the close button, for instance, without even needing to link the CloseButton.onPressed callback to the grid controller. Android's back button/gesture is another example of an action that causes the selection to be cleared due to the local history entry.

By the way, we need to add impliesAppBarDismissal: false to the LocalHistoryEntry, given that it will not dismiss the bar, at least in my case.

sounds reasonable. i'll expose such an attribute.

@hcbpassos hcbpassos self-assigned this Aug 29, 2023
@hcbpassos
Copy link
Owner

please, let me know if the last release works out for you :)

@iazel
Copy link
Author

iazel commented Aug 29, 2023

Thank you for updating it so soon, it works as expected and this helped me better understand LocalHistory 👍

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

No branches or pull requests

2 participants