Fix button opacity problem when saving personal details#10229
Merged
Conversation
Contributor
|
|
47 tasks
arosiclair
approved these changes
Aug 3, 2022
arosiclair
left a comment
Contributor
There was a problem hiding this comment.
LGTM though I'll let @tgolen chime in since he was looking at this more closely
Contributor
Author
|
Merging since this fix is pretty useful, still interested in feedback from @tgolen if you have any :D |
OSBotify
pushed a commit
that referenced
this pull request
Aug 8, 2022
Fix button opacity problem when saving personal details (cherry picked from commit f6cb210)
OSBotify
added a commit
that referenced
this pull request
Aug 8, 2022
…-10229 🍒 Cherry pick PR #10229 to staging 🍒
Contributor
|
🚀 Cherry-picked to staging by @Beamanator in version: 1.1.88-4 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
cc @tgolen since you were also looking into this solution yesterday
Details
The problem was, as mentioned here, some rerendering or gestures made the Opacity animation stop before it finished. In that comment I show some console logs that indicate the Animation never finished, so in this fix I make sure the Undim animation will always finish. It's possible we'll need the same for Dim in the future, but that's not something we need to worry about right now.
One alternate solution I had was to try to check if the value of
this.opacitywas < 1 & ifthis.props.shouldDimwasfalse, like this:However this doesn't work because
this.opacityis anAnimatedvalue - to get its value you have to do something hacky (see this SO)So instead of doing something hacky, we now just make sure it fully undims (becomes fully visible) when needed.
Fixed Issues
$ #10230
Tests
PR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */displayNamepropertythisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)QA Steps
Screenshots
Web
Screen.Recording.2022-08-03.at.1.04.18.PM.mov
Mobile Web
Screen.Recording.2022-08-03.at.1.21.17.PM.mov
Desktop
iOS
Screen.Recording.2022-08-03.at.1.44.13.PM.mov
Android
Screen.Recording.2022-08-03.at.1.51.48.PM.mov