You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Workspace - Room visibility options are also displayed as tooltips #34156
Version Number: 1.4.23.0 Reproducible in staging?: Y Reproducible in production?: No. new feature If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs:https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause - Internal Team Slack conversation:
👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:
Identify the pull request that introduced this issue and revert it.
// We only create tooltips for the first 10 users or so since some reports have hundreds of users, causing performance to degrade.
constshowTooltip=normalizedIndex<10;
What changes should be made to fix this?
We should add a prop to the SelectionList whether to show tooltips or not like shouldShowTooltip. Then we can disable the tooltip show prop in the ValueSelectorModal so that tooltip is hidden for value pickers.
Alternate solution
We can also add a shouldShowTooltip to the individual item itself so that we can get even greater control over which item to show tooltip and which not to (but I don't think such control is needed)
Activity
github-actions commentedon Jan 9, 2024
👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:
melvin-bot commentedon Jan 9, 2024
Triggered auto assignment to @Julesssss (
Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.esh-g commentedon Jan 9, 2024
This is not a blocker. Same behaviour on production as well
Screen.Recording.2024-01-09.at.9.18.09.PM.mov
esh-g commentedon Jan 9, 2024
Proposal
Please re-state the problem we are trying to solve
There should be no tooltip for items like pure text in
ValuePickerRoot cause
The root cause is that for every item in
SelectionList, we display the tooltip as long as it's index is less than 10.App/src/components/SelectionList/BaseSelectionList.js
Lines 304 to 305 in e9f27ab
What changes should be made to fix this?
We should add a prop to the
SelectionListwhether to show tooltips or not likeshouldShowTooltip. Then we can disable the tooltip show prop in theValueSelectorModalso that tooltip is hidden for value pickers.Alternate solution
We can also add a
shouldShowTooltipto the individualitemitself so that we can get even greater control over which item to show tooltip and which not to (but I don't think such control is needed)Julesssss commentedon Jan 9, 2024
Can confirm the above. Also, showing additional tooltips bad enough to block the deploy.
We should still fix
dukenv0307 commentedon Jan 9, 2024
Proposal
Please re-state the problem that we are trying to solve in this issue.
Room visibility options are also displayed as tooltips
What is the root cause of that problem?
We show the tooltip for 10 first items of seclection list
What changes do you think we should make in order to solve the problem?
#33580 in this PR, we're handling to remove the tooltip in the timezone page.
shouldShowTooltipprop to disable tooltip in the visibility as well.What alternative solutions did you explore? (Optional)
NA
situchan commentedon Jan 9, 2024
IMO, #33580 should cover this case as well
Julesssss commentedon Jan 9, 2024
Agree, thank you.