[HOLD for payment 2023-05-04] [$2000] No background color on hover on copy to clipboard in workspace->reimburse expenses

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Open the app
2. Open workspaces
3. Open any workspace
4. Open reimburse expenses
5. Hover on copy to clipboard icon
6. Get back to home page and open any report
7. Click on that report user profile to open user details page for that user
8. Hover on copy to clipboard icon

Expected Result:

Copy to clipboard in reimburse expenses should have background color effect on hover and properly spaced from the text.

Actual Result:

Inconsistent behavior for same copy to clipboard icon on hover between user details and reimburse expenses page

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• [ ] Android / native
• [ ] Android / Chrome
• [ ] iOS / native
• [ ] iOS / Safari
• [x] MacOS / Chrome / Safari
• [ ] MacOS / Desktop

Version Number: 1.2.90-4 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:

https://user-images.githubusercontent.com/43996225/228032891-b3b8c8d5-a029-4f4c-b151-4363c7eac2c7.mp4

https://user-images.githubusercontent.com/43996225/228032916-d571ba5e-0899-4320-b23d-361f9ecbd685.mp4

Expensify/Expensify Issue URL: Issue reported by: @dhanashree-sawant Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1679905653532999

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0125054b3431e347fc
• Upwork Job ID: 1643686746350624768
• Last Price Increase: 2023-04-19

[MelvinBot]

Triggered auto assignment to @mallenexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[MelvinBot]

Bug0 Triage Checklist (Main S/O)

• [x] This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• [x] This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• [x] This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• [x] This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• [x] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[mallenexpensify]

@dhanashree-sawant, before moving this along, can you do an audit to see if there are any other instances where there are inconsistencies with the way the copy button looks in the app? I'd prefer to fix multiple at once if possible vs creating multiple issues for the same inconsistency)

[dhanashree-sawant]

Hi @mallenexpensify , Sure will check throughout the app and let you know.

[dhanashree-sawant]

Hi @mallenexpensify, I have gone through all the pages and we only use copy to clipboard on message hover, user details and reimburse expenses. Only reimburse expenses page has the inconsistency which we will need to resolve.

[dhanashree-sawant]

Hi @mallenexpensify, recently we have again added new contact methods page in profile, even on that page we will need to resolve copy to clipboard icon issue

[MelvinBot]

@mallenexpensify Huh... This is 4 days overdue. Who can take care of this?

[MelvinBot]

@mallenexpensify 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[mallenexpensify]

Thanks @dhanashree-sawant , sorry for the delay, been sick the past week. Moving along.

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~0125054b3431e347fc

[MelvinBot]

Current assignee @mallenexpensify is eligible for the External assigner, not assigning anyone new.

[MelvinBot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @aimane-chnaif (External)

[MelvinBot]

Triggered auto assignment to @robertjchen (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[alitoshmatov]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Inconsistency in copy to clipboard buttons

What is the root cause of that problem?

This is caused due to different approaches in coding this buttons. The correct one, CommunicationsLink component is using ContextMenuItem like this https://github.com/Expensify/App/blob/0d2d52eaa4f5ce91585be007645ad1d4e6a230df/src/components/CommunicationsLink.js#L40-L47

Whereas in CopyTextToClipboard it is used like this: https://github.com/Expensify/App/blob/0d2d52eaa4f5ce91585be007645ad1d4e6a230df/src/components/CopyTextToClipboard.js#L62-L70

What changes do you think we should make in order to solve the problem?

We should update CopyTextToClipboard to use the same logic as CommunicationsLink

What alternative solutions did you explore? (Optional)

[alitoshmatov]

Updated code reference

[aimane-chnaif]

@alitoshmatov thanks for your proposal but that will easily break style inside text.

For contributors:

• please explain in detail how to prevent style break in inline text
• please share 2 videos at the end of your proposals - one for web, one for native Thanks

[allroundexperts]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The copy to clipboard button is not hovered.

What is the root cause of that problem?

The root cause of the issue is that we're not wrapping the Icon inside the button inside a Pressable component. Not doing so does not result in a hover.

What changes do you think we should make in order to solve the problem?

We can replace this block with the following:

<View>
                        <Pressable
                            focusable
                            onPress={this.copyToClipboard}
                            style={({hovered, pressed}) => ([
                                StyleUtils.getButtonBackgroundColorStyle(getButtonState(hovered, pressed, this.state.showCheckmark), true),
                                {marginLeft: 4},
                            ])}
                        >
                            {({hovered, pressed}) => (
                                <Icon
                                    src={this.state.showCheckmark ? Expensicons.Checkmark : Expensicons.Copy}
                                    fill={StyleUtils.getIconFillColor(getButtonState(hovered, pressed, this.state.showCheckmark))}
                                    width={variables.iconSizeSmall}
                                    height={variables.iconSizeSmall}
                                    inline
                                />
                            )}
                        </Pressable>
                    </View>

The position of the Pressable component dictates the style breaking of the inline text here. If we add it such that it wraps around the email address as well, then the inline styles break. However, for our case, I think its sufficient to add the Pressable around the Icon only. This does not break the layout on any of the platforms.

Result

https://user-images.githubusercontent.com/30054992/230190034-707e6203-9056-411e-8424-1e5c391068f8.mov

https://user-images.githubusercontent.com/30054992/230190174-bfed227f-a8e5-46ff-938e-fe04b95f0cc7.mov

https://user-images.githubusercontent.com/30054992/230190194-3afb52cc-6753-4f57-a610-372db6973373.mov

https://user-images.githubusercontent.com/30054992/230190235-3b60094f-1412-4ad0-9184-3df00e9cbd3e.mov

https://user-images.githubusercontent.com/30054992/230193212-75d57cfa-92c4-4d0d-a424-3fa0acfa71b4.mov

What alternative solutions did you explore? (Optional)

None

[soumyajit4419]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The copy to clipboard button is not hovered.

What is the root cause of that problem?

The root cause of the issue is that there is no hover functionality present in copyToClipboard component.

What changes do you think we should make in order to solve the problem?

We can wrap the icon present in the copyToClipboard component with a view and track the mouseEvent to maintain the hover state. We can replace this Block with below function.

    <View
        style={[
            Platform.OS === 'web' && styles.copyToClipBoardButton,
            styles.alignItemsCenter, styles.justifyContentCenter,
            this.state.isHovered ? styles.hoveredButton : styles.bgTransparent,
        ]}
        onMouseEnter={
            () => {
                this.setState({isHovered: true});
            }
          }
        onMouseLeave={
            () => {
                this.setState({isHovered: false});
            }
          }
    >
        <Icon
            src={this.state.showCheckmark ? Expensicons.Checkmark : Expensicons.Clipboard}
            fill={this.state.showCheckmark ? themeColors.iconHovered : themeColors.icon}
            width={variables.iconSizeSmall}
            height={variables.iconSizeSmall}
            inline
        />
    </View>

Result

https://user-images.githubusercontent.com/46092576/230205878-94772800-23c6-4a6f-a4f6-10a547a3390a.mov

https://user-images.githubusercontent.com/46092576/230205635-24b8e662-4926-41cc-8504-913d7434e02c.mov

https://user-images.githubusercontent.com/46092576/230205773-0cd6a3f6-58ae-40fe-b82f-020fa483fa81.mov

What alternative solutions did you explore? (Optional)

NA

[robertjchen]

Thanks for the proposals and detailed screen recordings on all the platforms! 🙇

At the moment, I'm partial to @allroundexperts 's proposal https://github.com/Expensify/App/issues/16574#issuecomment-1498029836 since it doesn't involve any platform specific logic as in the second proposal. What do you think @aimane-chnaif ? 🙏

[aimane-chnaif]

For confirmation: we don't need to show circle background in wrapper when hovered, right? Correct I believe because if we add it, there should be space between email and copy icon.

[robertjchen]

we don't need to show circle background in wrapper when hovered, right?

Ah, I was thinking of that as well. To maintain consistency in other places, I'd say it should also show a background when hovered 👍

[aimane-chnaif]

Ah, I was thinking of that as well. To maintain consistency in other places, I'd say it should also show a background when hovered 👍

ok then no acceptable proposal so far

[allroundexperts]

@aimane-chnaif @robertjchen We can add a background as well using the same approach in my proposal by wrapping the Icon in a View but IMO it does not look good. We should have added the background IF there was no text after the icon. In this case, however, there is text after the Icon.

https://user-images.githubusercontent.com/30054992/230788698-e9259558-4365-4471-9276-6a7627d823c6.mov

[aimane-chnaif]

I had same opinion as @allroundexperts.

I'd like to get what design team thinks about this inconsistency. cc: @shawnborton

[allroundexperts]

Just as an FYI, we're using the same component below as well.

[aimane-chnaif]

Just as an FYI, we're using the same component below as well.

yup, these are all pages which have copy icon: https://github.com/Expensify/App/pull/17008#pullrequestreview-1375634404

[MelvinBot]

@robertjchen @mallenexpensify @aimane-chnaif this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[MelvinBot]

@robertjchen @mallenexpensify @aimane-chnaif this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[MelvinBot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[MelvinBot]

Upwork job price has been updated to $2000