Skip to content
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

[$500][Image] Web - Profile - The avatar is highlighted with a blue frame #10599

Closed
kbecciv opened this issue Aug 26, 2022 · 58 comments
Closed
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@kbecciv
Copy link

kbecciv commented Aug 26, 2022

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:

  1. Go to URL https://staging.new.expensify.com/
  2. Go to Profile>Upload photo
  3. Choose the photo>Open
  4. Press "Esc" key

Expected Result:

After canceling the upload of the photo using the Esc key, the avatar should not be highlighted with a blue frame

Actual Result:

After canceling the photo upload using the Esc key, the avatar is highlighted with a blue frame

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number: 1.1.91.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers): any

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Bug5705026_Recording__1635.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

@melvin-bot
Copy link

melvin-bot bot commented Aug 26, 2022

Triggered auto assignment to @NikkiWines (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@varshamb
Copy link
Contributor

varshamb commented Aug 26, 2022

Proposal

Use PressableWithoutFocus in AvatarWithImagePicker.

@parasharrajat
Copy link
Member

@varshamb Please go through our contribution policy. We don't accept proposals before issue is exported.

@varshamb
Copy link
Contributor

@parasharrajat
I read the below in contribution policy and interpreted that it is allowed to Propose even before Label External. Please correct me if I am wrong.

image

@parasharrajat
Copy link
Member

parasharrajat commented Aug 26, 2022

Yeah, you can but we don't accept/approve/hire the proposals before the issue is made external, also we can use the proposed solution if the proposal is posted before an issue is made external. I thought to inform you about the policy as I saw you proposing on a couple of issues in this order and being a new contributor.

@varshamb
Copy link
Contributor

@parasharrajat Thanks.

@melvin-bot melvin-bot bot added the Overdue label Aug 29, 2022
@NikkiWines
Copy link
Contributor

Able to reproduce this on staging, looks like it's good for an external issue 👍

@melvin-bot melvin-bot bot removed the Overdue label Aug 29, 2022
@NikkiWines NikkiWines added the External Added to denote the issue can be worked on by a contributor label Aug 29, 2022
@melvin-bot
Copy link

melvin-bot bot commented Aug 29, 2022

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

@melvin-bot
Copy link

melvin-bot bot commented Sep 2, 2022

Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Sep 2, 2022
@melvin-bot
Copy link

melvin-bot bot commented Sep 6, 2022

Still overdue 6 days?! Let's take care of this!

@melvin-bot
Copy link

melvin-bot bot commented Sep 8, 2022

Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

@aimane-chnaif
Copy link
Contributor

Proposal

<Pressable
onPress={() => this.setState({isMenuVisible: true})}
>

Add this props to Pressable component:

                    onMouseDown={e => e.preventDefault()}

demo video: https://drive.google.com/file/d/1qXw3l1MhKIQqcfDytCFJZuFEYamqX-Fl/view?usp=sharing

@melvin-bot
Copy link

melvin-bot bot commented Sep 12, 2022

12 days overdue. Walking. Toward. The. Light...

@melvin-bot melvin-bot bot removed the Daily KSv2 label Sep 15, 2022
@melvin-bot
Copy link

melvin-bot bot commented Sep 15, 2022

This issue has not been updated in over 14 days. eroding to Weekly issue.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Overdue labels Sep 15, 2022
@varshamb
Copy link
Contributor

@NikkiWines @JmillsExpensify no one is assigned to this issue

@NikkiWines
Copy link
Contributor

Ah, sorry, this is my bad. Unassigned myself too quickly after the External label was assigned so it also unassigned @JmillsExpensify. Re-applying the label.

@NikkiWines NikkiWines removed the External Added to denote the issue can be worked on by a contributor label Sep 19, 2022
@mallenexpensify mallenexpensify added Help Wanted Apply this label when an issue is open to proposals by contributors Daily KSv2 and removed Monthly KSv2 labels Nov 1, 2022
@mallenexpensify mallenexpensify changed the title [$500] Web - Profile - The avatar is highlighted with a blue frame [$500][Image] Web - Profile - The avatar is highlighted with a blue frame Nov 1, 2022
@Beamanator
Copy link
Contributor

Yep thanks @mallenexpensify 👍

@melvin-bot melvin-bot bot added the Overdue label Nov 3, 2022
@mallenexpensify
Copy link
Contributor

Doubled price two days ago, not rushing this one, it's a minor bug

@melvin-bot melvin-bot bot removed the Overdue label Nov 4, 2022
@railway17
Copy link
Contributor

railway17 commented Nov 4, 2022

Proposal

I read the issue description and could reproduce it on my side.

Blue border is displaying because we have the below box-shadow CSS in the index.html

        :focus-visible {
            outline: 0;
            box-shadow: inset 0px 0px 0px 1px #0185ff;
        }

image

So, we can fix this issue in 3 ways.

  1. We can remove this style from index.html (But I don't think it is good way because it may be used in other components)
  2. We can add the inline style like below in src/components/AvatarWithImagePicker.js
        return (
            <View style={[styles.alignItemsCenter, ...additionalStyles]}>
                <Pressable
+                    style={[styles.noSelect]}
                    onPress={() => this.setState({isMenuVisible: true})}
                >
                    <View style={[styles.pRelative, styles.avatarLarge]}>
  1. We may use PressableWithoutFocus component instead of Pressable like below
         return (
             <View style={[styles.alignItemsCenter, ...additionalStyles]}>
-                <Pressable
+                <PressableWithoutFocus
                     onPress={() => this.setState({isMenuVisible: true})}
                 >
                     <View style={[styles.pRelative, styles.avatarLarge]}>
...
                             )}
                         </AttachmentPicker>
                     </View>
-                </Pressable>
+                </PressableWithoutFocus>
                 <ConfirmModal
                     title={this.state.errorModalTitle}
                     onConfirm={this.hideErrorModal}

Please review these solutions and let me know which one will be better for you.
Thank you

@parasharrajat
Copy link
Member

I will check the proposal as soon as possible.

@melvin-bot melvin-bot bot added the Overdue label Nov 7, 2022
@mallenexpensify
Copy link
Contributor

@parasharrajat please review @railway17 's proposal when you're able

@melvin-bot melvin-bot bot removed the Overdue label Nov 7, 2022
@railway17
Copy link
Contributor

railway17 commented Nov 8, 2022

@parasharrajat
Could you kindly review my proposal today?
I have an idea for another issue, but still waiting for your review because I can't submit multiple proposals at a time.
And hopefully, let me know how I can become an experienced contributor to work for multiple jobs simultaneously. 🙈
Thank you.

@parasharrajat
Copy link
Member

parasharrajat commented Nov 8, 2022

how I can become an experienced contributor

There is still time 😄. You have only completed one task so far. You are free to send proposals to multiple issues.

Yeah, reviewing it now.

@railway17
Copy link
Contributor

how I can become an experienced contributor

There is still time 😄. You have only completed one task so far. You are free to send proposals to multiple issues.

No, I've completed 2 tasks (but I know it is not much different from 1 task completion 😄)

@parasharrajat
Copy link
Member

@railway17 your proposal matches the old proposals. This can't be considered valid.

@parasharrajat
Copy link
Member

Overall, this might be the right time to look back at the old solutions and the current state of the app to understand what can be done to fix this issue.

My suggestion is that you test your solution thoroughly and suggest alternatives to problems that they cause or find a solution to overcome those as well.

  1. Tab navigation focus is useful.
  2. Is there a solution that can be used to fix this issue without breaking the tab navigation focus?

Let's discuss this to reach a conclusion.

@railway17
Copy link
Contributor

@railway17 your proposal matches the old proposals. This can't be considered valid.

hmm... I think I wrote the root cause and solution.
Also, also think it is the simplest solution.
But if you still don't think it is the correct solution, I will leave for this issue.

@parasharrajat
Copy link
Member

parasharrajat commented Nov 8, 2022

Mainly a proposal should not match with existing ones. Even if we decide to go with an old posted solution, the Original author will be hired to work on that.

@mallenexpensify
Copy link
Contributor

@railway17 specifically from CONTRIBUTING.md

Note: Before submitting a proposal on an issue, be sure to read any other existing proposals. Any new proposal should be substantively different from existing proposals.

@melvin-bot melvin-bot bot added the Overdue label Nov 11, 2022
@aldo-expensify
Copy link
Contributor

Having a look at this issue again, I'm not really convince about doing anything... why do we think there shouldn't be a blue frame if the focus is in the avatar? it is an input after all. If you tab around that page and press enter when the blue frame appears in the avatar, you get the modal to upload a new picture, which makes 100% sense to me. If we remove the blue frame, how would the user know that the avatar is focused and that pressing enter will make the modal appear?

@melvin-bot melvin-bot bot removed the Overdue label Nov 11, 2022
@JmillsExpensify
Copy link

Great point. I agree with you.

@aldo-expensify
Copy link
Contributor

Great, closing then :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests