hcbpassos / drag_select_grid_view

A grid that supports both dragging and tapping to select its items.
BSD 2-Clause "Simplified" License
133 stars 34 forks source link

Cancel through SelectionController mess up LocalHistory #31

Closed iazel closed 1 year ago

iazel commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

iazel commented 1 year ago

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