-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[Hold for payment 2023-07-05][$1000] copy to clipboard tooltip is missing in Contact methods #21074
Comments
Triggered auto assignment to @zanyrenney ( |
Bug0 Triage Checklist (Main S/O)
|
ProposalPlease re-state the problem that we are trying to solve in this issue.copy to clipboard tooltip is missing in Contact methods page when we hover over the copy icon next to the email address What is the root cause of that problem?Root cause of the problem is the changes happened in past with the App/src/components/CopyTextToClipboard.js Lines 29 to 38 in 297e5aa
Upon introduction of App/src/components/PressableWithDelayToggle.js Lines 94 to 126 in 297e5aa
What changes do you think we should make in order to solve the problem?We need to update the <PressableView
style={[styles.flexRow, ...props.styles]}
onPress={updatePressState}
>
<>
<Text
suppressHighlighting
style={[styles.mr1, ...props.textStyles]}
>
{props.isDelayButtonStateComplete && props.textChecked ? props.textChecked : props.text}
</Text>
<Tooltip
containerStyles={[styles.flexRow]}
text={props.isDelayButtonStateComplete ? props.tooltipTextChecked : props.tooltipText}
>
<Pressable
ref={props.innerRef}
focusable
accessibilityLabel={props.isDelayButtonStateComplete ? props.tooltipTextChecked : props.tooltipText}
onPress={updatePressState}
>
{({hovered, pressed}) => (
<>
{props.icon && (
<Icon
src={props.isDelayButtonStateComplete ? props.iconChecked : props.icon}
fill={StyleUtils.getIconFillColor(getButtonState(hovered, pressed, props.isDelayButtonStateComplete))}
style={props.iconStyles}
width={variables.iconSizeSmall}
height={variables.iconSizeSmall}
/>
)}
</>
)}
</Pressable>
</Tooltip>
</>
</PressableView> NOTE: adding the below information later, and no other proposal has mentioned this till the time of edit EDIT: The above change is consistent with the prior version of CopyTextToClipboard which used to be there before we introduced source: App/src/components/CopyTextToClipboard.js Lines 45 to 66 in 99bbff8
Resultsbandicam.2023-06-12.19-48-20-113.mp4What alternative solutions did you explore? (Optional)None |
ProposalPlease re-state the problem that we are trying to solve in this issue.Tooltip doesn't show in the contact method copy text to clipboard. What is the root cause of that problem?CopyTextToClipboard uses PressableWithDelayToggle. In PressableWithDelayToggle, we have a Tooltip that wraps a React.Fragment. App/src/components/PressableWithDelayToggle.js Lines 90 to 128 in f0c6631
Why doesn't it work? Tooltip internally uses Hoverable. To make Hoverable work, the children should be able to accept both the What changes do you think we should make in order to solve the problem?
|
Yep fair, i can reproduce and this seems like a sensible addition to contact methods. though should it only apply to email addresses or also to phone numbers seeing as both will be available as contact methods? |
Job added to Upwork: https://www.upwork.com/jobs/~014363b93cc961b40f |
Current assignee @zanyrenney is eligible for the External assigner, not assigning anyone new. |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @narefyev91 ( |
Here is my suggestion: To fix the issue where the copy to clipboard tooltip is not displaying, you can try the following steps: Verify Tooltip Configuration: Ensure that the text prop provided to the Tooltip component contains the appropriate value for the copy to clipboard functionality. Double-check that props.tooltipTextChecked and props.tooltipText are correctly set. Check Button State: Confirm that the isDelayButtonStateComplete prop is correctly indicating the state of the button. If this prop is not being set or evaluated correctly, it may impact the display of the tooltip. Review Icon Configuration: Make sure that props.iconChecked and props.icon are set correctly and that the icons are being rendered conditionally based on the button state. Inspect Component Styling: Check if there are any conflicting or overriding styles applied to the Tooltip component or its parent components. Ensure that the containerStyles prop provided to the Tooltip component is not interfering with the tooltip's visibility. Verify Accessibility Properties: Double-check the accessibility properties such as focusable and accessibilityLabel provided to the Pressable component. These properties should accurately reflect the button's functionality and state. <PressableView style={[styles.flexRow, ...props.styles]} onPress={updatePressState}> |
📣 @kevinleerongordon! 📣
|
I can able to replicate the issues and I find the reason for this issue. Issue: Reason for the issues Fixes
Issues fixes link Contributor details |
📣 @staticGuru! 📣
|
Contributor details |
ProposalPlease re-state the problem that we are trying to solve in this issue. What is the root cause of that problem?
What changes do you think we should make in order to solve the problem?
|
📣 @devhd9! 📣
|
Contributor details |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
you made a PR @staticGuru ? |
Proposal of @chiragxarora #21074 (comment) looks good for me - it's correctly indicate the issue - and mostly suggesting to have the same structure of component which we have before inside CopyTextToClipboard |
Triggered auto assignment to @bondydaa, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
okay i believe all that happens is we add the |
Thanks @bondydaa ! |
Hey @chiragxarora I don't see that you applied for the job on Upwork as was requested in this comment. I am manually sending you an invite. Also invited the reporter - Manan Gadhiya. I believe @narefyev91 is paid outside Upwork! Let me know if I've gotten any of that wrong! |
invite accepted @zanyrenney |
Thank you! |
@zanyrenney applied, thanks! |
Hi @zanyrenney |
@bondydaa please can you un-assign and then re-assign @chiragxarora as I keep getting this error and I think it might be due to the automation as @chiragxarora said. Thanks! |
BUMP: @bondydaa |
hmm sorry why does it matter who unassigns/re-assigns? pretty sure zany could have and it would have worked just fine too. |
📣 @chiragxarora 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Upwork job |
📣 @gadhiyamanan 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app! |
@zanyrenney I've received the offer, can you please approve the milestones as this one is already way past the regression period? |
Paid for Manan and Chirag, other contributor paid outside GH so we are all set here, closing! |
@zanyrenney , this GH qualifies for urgency bonus too, please check |
|
bump: @zanyrenney |
@zanyrenney could you pls check this when you find time? |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Action Performed:
Expected Result:
should show copy to clipboard tooltip
Actual Result:
does not show copy to clipboard tooltip
Workaround:
Can the user still use Expensify without this being fixed? Have you informed them of the workaround?
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.29-0
Reproducible in staging?: y
Reproducible in production?: y
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
Notes/Photos/Videos: Any additional supporting documentation
Screen.Recording.2023-06-12.at.4.33.43.PM.mov
Recording.1029.mp4
Expensify/Expensify Issue URL:
Issue reported by: @gadhiyamanan
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1686567859832639
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: