Fix dependent tag second-level row stuck faded after selecting first-level tag#94756
Draft
MelvinBot wants to merge 1 commit into
Draft
Fix dependent tag second-level row stuck faded after selecting first-level tag#94756MelvinBot wants to merge 1 commit into
MelvinBot wants to merge 1 commit into
Conversation
…hlight style Co-authored-by: Bernhard Owen Josephus <bernhardoj@users.noreply.github.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
bernhardoj
reviewed
Jun 28, 2026
| // The animated highlight style is always applied (never swapped on `highlighted`) so the entry fade is never | ||
| // detached from the node mid-animation. When `highlighted` is false the hook returns a static opacity: 1, so | ||
| // this is inert for non-highlighted rows. See https://github.com/Expensify/App/issues/94005. | ||
| outerWrapperStyle={[outerWrapperStyle, animatedHighlightStyle]} |
Contributor
There was a problem hiding this comment.
@MelvinBot No need for a comment here and instead of rendering a MenuItem, let's render MenuItemWithTopDescription here.
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.
Explanation of Change
When a dependent (multi-level) tag's first level is selected, the newly-revealed second-level tag row (e.g.
Regionafter selectingState) appeared empty/faded on iOS and Android until the user tapped it.Root cause:
MenuItemWithTopDescriptionapplied the entry-fade animation style only while itshighlightedprop wastrue(outerWrapperStyle={highlighted ? highlightedOuterWrapperStyle : outerWrapperStyle}). For a freshly-revealed dependent tag row,highlightedflipstrue → falseon the very next render — mid-animation — which detaches the animated highlight style from the native node before the fade'swithTimingcompletes. The shared value keeps animating to1on the UI thread, but nothing reads it anymore, so the row freezes at a partial opacity. Web was unaffected because it re-applies the flattened style on every React commit.Fix: Extracted the highlight into a dedicated
HighlightableMenuItemWithTopDescriptionthat owns theuseAnimatedHighlightStylehook and always applies the animated style (no longer gated onhighlighted), so the style stays attached for the entire fade. Whenhighlightedisfalse, the hook returns a staticopacity: 1with no pulse (initialNonRepeatableProgressValue = 1), so the always-applied style is inert for non-highlighted rows.MenuItemWithTopDescriptionis reverted to a plain pass-through, leaving a single highlight implementation.Migrated the three remaining
MenuItemWithTopDescriptioncall sites that passhighlighted:MoneyRequestConfirmationList/sections/TagFields.tsx(the reported bug)pages/iou/SplitExpenseEditPage.tsx(same dependent-tag pattern)ReportActionItem/MoneyRequestView.tsxThe
WorkspaceInitialPage/DomainInitialPagecall sites already use the separateHighlightableMenuItemcomponent (which already applies its highlight style unconditionally), so they needed no change.Fixed Issues
$ #94005
PROPOSAL: #94005 (comment)
Tests
Offline tests
Same as Tests.
QA Steps
Same as Tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionScreenshots/Videos
/** comment above it */thisproperly 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)Screenshots/Videosundefined