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

[Feature] Add Alert When Network Lost #328

Merged
merged 24 commits into from
Sep 1, 2022

Conversation

jpeng-ms
Copy link
Member

@jpeng-ms jpeng-ms commented Aug 30, 2022

Purpose

The goal is to alert Contoso user they cannot join the call if their internet is down.

Technical details

This PR introduces a network manager which listens to system event on network condition.
Setupviewmodel would be responsible to dispatch error actions to update network state, which would be used by error banner view accordingly.

The overall flow is the following:

in the event of network lost:

nothing would happen until user tap on "join call" button:

1. setup view model would ask manager if network is online

2. if network is down, dispatch an error action with type .connectionFailed

3. then it would be handled by error reducer

4. error state would be updated

5. error banner view model which listens to error state changes would then display error without any involvement of set up view model or network manager.

Does this introduce a breaking change?

[ ] Yes
[X] No

Pull Request Type

What kind of change does this Pull Request introduce?

[ ] Bugfix
[X] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

How to Test

  1. clone the repo
  2. set target to demo app
  3. select a physical device
  4. click run

What to Check

  1. when there's no active internet connection, an alert should be shown when you tap on "Join Call"

Other Information

  1. The scope of this ticket is limited to set up view only and we should not handle anything on calling view as it's calling SDK's task to let us know about call conditions.
  2. Please use physical device for testing. The network changes on your laptop does not propagate to simulator properly and you might notice unexpected behaviour due to this. 3.

Expected Behaviour

  1. launch composite + network down = no error banner
  2. tap on "join call" + network down = error banner shown
  3. dismiss error banner + tap on "join call" + network down = new error banner shown
  4. error banner shown + tap on "join call" + network down = no action
  5. error banner shown + tap on "join call" + network back online = error banner remains then gets dismissed when join call button changes to "joining the call..." with a loading spinner.
  6. error banner shown + state changes (change audio/speaker/camera, enter background/foreground, swipe down notification, etc.) = error banner remains no action
  7. dismiss error banner + state changes = no action, error banner stays hidden
  8. SDK error + network down = error banner would take SDK error unless user taps on "join call" then SDK error would be replaced to network offline.

Screenshots

SDK error when network is offline (without tapping on join call):
image
SDK error when network is offline (after tapping on join call):
image
UI:
image

@acs-ui-bot acs-ui-bot added the feature For new features intended for a future release label Aug 30, 2022
@jpeng-ms jpeng-ms changed the title [Feature] add alert when network lost [Feature] Add Alert When Network Lost Aug 30, 2022
@jpeng-ms jpeng-ms marked this pull request as ready for review August 30, 2022 18:47
@jpeng-ms jpeng-ms requested review from a team as code owners August 30, 2022 18:47
Copy link
Contributor

@kevinyulu kevinyulu left a comment

Choose a reason for hiding this comment

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

tested on device LGTM

@jpeng-ms jpeng-ms merged commit 5d6ffc4 into develop Sep 1, 2022
@jpeng-ms jpeng-ms deleted the feature/add-alert-when-network-lost branch September 1, 2022 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature For new features intended for a future release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants