Skip to content
This repository was archived by the owner on May 1, 2024. It is now read-only.

Fix DisplaySnackBarAsync Task never completes if another SnackBar appears, and completes before SnackBar action #1357

Merged
merged 31 commits into from
Nov 25, 2021
Merged

Fix DisplaySnackBarAsync Task never completes if another SnackBar appears, and completes before SnackBar action #1357

merged 31 commits into from
Nov 25, 2021

Conversation

maxkoshevoi
Copy link
Contributor

@maxkoshevoi maxkoshevoi commented Jun 2, 2021 •
edited
Loading

Description of Change

  • Set task result as false if SnackBar was closed via anything but executing an action.
  • Wait until SnackBar action completion and pass exceptions from it into resulting task so they can be caught

Issue
There are 4 possible ways for SnackBar to be closed:
image

Current implementation handles only two (so task never completed for other two):
image

Bugs Fixed

API Changes

None

Behavioral Changes

  • SnackBar task always competes
  • DisplaySnackBarAsync waits until SnackBar action completes
  • Catching exceptions in DisplaySnackBarAsync also catches them in SnackBar action

PR Checklist

  • Has a linked Issue, and the Issue has been approved
  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard

@maxkoshevoi maxkoshevoi mentioned this pull request Jun 2, 2021
Closed
@maxkoshevoi maxkoshevoi changed the title Fix DisplaySnackBarAsync Task never completes if another SnackBar appears Fix DisplaySnackBarAsync Task never completes if another SnackBar appears, and completes before SnackBar action Jun 2, 2021
@maxkoshevoi maxkoshevoi mentioned this pull request Jun 2, 2021
Closed
@pictos
Copy link
Contributor

pictos commented Jun 17, 2021

@maxkoshevoi are you able to create tests for it inside our sample in the TestCases?

@jfversluis jfversluis added the needs-tests PR that could use some tests to be added before being merged label Jun 18, 2021
@maxkoshevoi
Copy link
Contributor Author

maxkoshevoi commented Jun 18, 2021 •
edited
Loading

@pictos, @jfversluis Added tests

@maxkoshevoi
Copy link
Contributor Author

@jfversluis @pictos Could you take another look at this, please?

@Dreamescaper
Copy link

@jfversluis @pictos @VladislavAntonyuk
Guys, is there anything that stops this PR from being merged? It have to add ugly workarounds due to this issue, I'd really like to remove them :)

@VladislavAntonyuk
Copy link
Contributor

@Dreamescaper if I had rights, I would merge this PR

pictos
pictos previously requested changes Oct 22, 2021
View reviewed changes
@maxkoshevoi maxkoshevoi requested a review from pictos October 29, 2021 10:02
@TheCodeTraveler
Copy link
Contributor

@Dreamescaper if I had rights, I would merge this PR

@VladislavAntonyuk Go for it!

It looks like it needs one more Review + Approve then you can merge it.

Give it a quick double-check to make sure everything still looks good, approve it, then merge it in!

@TheCodeTraveler TheCodeTraveler added this to the 1.4 milestone Nov 24, 2021
@VladislavAntonyuk VladislavAntonyuk enabled auto-merge (squash) November 25, 2021 12:10
@VladislavAntonyuk
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@VladislavAntonyuk VladislavAntonyuk merged commit fe2aeb2 into xamarin:main Nov 25, 2021
@maxkoshevoi maxkoshevoi deleted the 1341-infinite-toast branch November 25, 2021 13:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@VladislavAntonyuk VladislavAntonyuk VladislavAntonyuk approved these changes

@pictos pictos Awaiting requested review from pictos

Assignees
No one assigned
Labels
a/Snackbar needs-tests PR that could use some tests to be added before being merged
Projects
None yet
Milestone
2.0.0
Development

Successfully merging this pull request may close these issues.

[Bug] DisplaySnackBarAsync Task never completes if another SnackBar appears [Bug] DisplaySnackBarAsync does not wait for action task
7 participants
@maxkoshevoi @pictos @Dreamescaper @VladislavAntonyuk @TheCodeTraveler @jfversluis @jsuarezruiz