Fix bug with flickering pop-up #58539
Conversation
| dateProgressTableInvitationDelayed === undefined || | ||
| hasSeenProgressTableInvite === undefined | ||
| ) { | ||
| // Do not proceed if data has not been fully loaded. |
There was a problem hiding this comment.
Intentionally left this comment in here.
There was a problem hiding this comment.
Note that once this data is fully loaded (for even a brand new user), the values are set to null and false
| @@ -35,7 +35,10 @@ describe('UnconnectedInviteToV2ProgressModal', () => { | |||
| } | |||
|
|
|||
| it('renders the dialog with required elements', () => { | |||
| renderDefault(); | |||
| renderDefault({ | |||
| hasSeenProgressTableInvite: false, | |||
There was a problem hiding this comment.
Should we just add these to DEFAULT_PROPS?
| @@ -141,8 +141,6 @@ describe('SectionProgressSelector', () => { | |||
| it('sets user preference when link clicked', () => { | |||
| renderDefault(); | |||
|
|
|||
| const remindLaterLink = screen.getByText('Remind me later'); | |||
There was a problem hiding this comment.
Why are we removing this?
There was a problem hiding this comment.
In the original tests we added this in because the pop-up got in the way of viewing the screen with the SectionProgressSelector in it. With the changes to how the pop-up is rendered, it will not render the pop-up until it has data back from Redux about if the pop-up hasBeeenSeen or if there is a dateLastDelayed. We could make add these reducers to this test, but given that this test is more about testing the SectionProgressSelector and we already have eyes tests that ensure that this works, I was thinking that wasn't necessary. Thoughts?
Liam noticed that when the progress page re-loaded, the invitation to see the new progress view would flicker open.
Screen.Recording.2024-05-10.at.10.08.25.AM.mov
This PR addresses that issue.
Links
Testing story
Deployment strategy
Follow-up work
Privacy
Security
Caching
PR Checklist: