Fixes Crash On Double Click Of Space FABs #7102
Conversation
| // Click action for open spaces modal goes here | ||
| spaceListBottomSheet.show(requireActivity().supportFragmentManager, SpaceListBottomSheet.TAG) | ||
| views.newLayoutOpenSpacesButton.debouncedClicks { | ||
| spaceListBottomSheet.takeIf { !it.isAdded }?.show(requireActivity().supportFragmentManager, SpaceListBottomSheet.TAG) |
There was a problem hiding this comment.
I think the root cause of the issue is to instantiate the BottomSheets in advance at this point. I would have fixed this issue differently. Using debouncedClicks is OK, but using takeIf { !it.isAdded } looks a bit like a workaround.
WDYT?
There was a problem hiding this comment.
Also there is no need to inject the constructor here.
There was a problem hiding this comment.
I believe the approach you're leading towards is to instantiate the bottom sheet as we show it? (SpaceListBottomSheet.show())
I considered this approach but doing so would just reduce the bug from a crash to having two bottom sheets show together. debouncedClicks may help with this, but I wanted to add the !it.isAdded check as a guaranteed method.
Research also shows me that making this check (or using bottomSheet.isShowing in other cases) is a common practice.
There was a problem hiding this comment.
OK, thanks for cleaning the constructor.
There was a problem hiding this comment.
One more point:
When spaceListBottomSheet is displayed, if the user rotates it's device, the Fragment SpaceListBottomSheet will be restored by the System (the BottomSheet will still be visible), but NewHomeDetailFragment will not have a correct reference to this Fragment, since it will just instantiate a new instance in the constructor.
will not work (I did not check though).
You should try to get the reference (by tag for instance) to the restored bottom sheet Fragment instead of creating a new one.
|
Kudos, SonarCloud Quality Gate passed! |
Type of change
Content
Fixes Crash On Double Click Of Space FABs
Motivation and context
Fixes #7087
Screenshots / GIFs
N/A
Tests
Double quick really quickly on the space list or new chat FABs in the new layout. These would have previously crashed the app.
Tested devices
Checklist