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

View event replay #7917

Merged
merged 2 commits into from
Jan 10, 2023
Merged

View event replay #7917

merged 2 commits into from
Jan 10, 2023

Conversation

bmarty
Copy link
Member

@bmarty bmarty commented Jan 9, 2023

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

#7724 introduced a regression, especially on the login screen.
In case of error, a ViewEvent can be triggered. But next Fragment to be displayed will also handle this Event, which is not expected.

The change in this PR ensure that Event older than 150ms cannot be handled.

Navigation Event triggered when the app is in background can be ignored due to this change, but this is an edge case, and we do not want to do a bigger rework of this part on the code base right now, and fixing the issue is more important.

Motivation and context

Mainly avoid displaying several time the same issue during the sign up / sign in flow.

Screenshots / GIFs

Tests

  • Step 1
  • Step 2
  • Step ...

Tested devices

  • Physical
  • Emulator
  • OS version(s):

Checklist

@bmarty bmarty added the Z-NextRelease For issues and PRs which should be included in the NextRelease. label Jan 9, 2023
@bmarty bmarty requested review from a team and jmartinesp and removed request for a team January 9, 2023 17:01
@ElementBot
Copy link

Warnings
⚠️

Please add a changelog. See instructions here

Generated by 🚫 dangerJS against 1c51eda

@ElementBot
Copy link

Warnings
⚠️

Please add a changelog. See instructions here

Generated by 🚫 dangerJS against 8bbd88f

@bmarty bmarty force-pushed the feature/bma/viewEventReplay branch from 8bbd88f to 9b421e2 Compare January 10, 2023 09:40
Copy link
Member

@jmartinesp jmartinesp left a comment

Choose a reason for hiding this comment

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

LGTM, just a minor comment about documenting the workaround.

@ElementBot
Copy link

Warnings
⚠️

Please add a changelog. See instructions here

⚠️ Please add a reviewer to your PR.

Generated by 🚫 dangerJS against 9b421e2

@bmarty bmarty force-pushed the feature/bma/viewEventReplay branch from 9b421e2 to 02c61d3 Compare January 10, 2023 09:52
@ElementBot
Copy link

Warnings
⚠️

Please add a changelog. See instructions here

⚠️ Please add a reviewer to your PR.

Generated by 🚫 dangerJS against 02c61d3

@sonarcloud
Copy link

sonarcloud bot commented Jan 10, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

90.0% 90.0% Coverage
0.0% 0.0% Duplication

Copy link
Member

@ganfra ganfra left a comment

Choose a reason for hiding this comment

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

Better like this, thanks!

@bmarty bmarty merged commit ac482b1 into develop Jan 10, 2023
@bmarty bmarty deleted the feature/bma/viewEventReplay branch January 10, 2023 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Z-NextRelease For issues and PRs which should be included in the NextRelease.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants