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

[$250] Web - Copilot - "Verify" button position is inconsistent for the same step in different flow #52127

Closed
1 of 8 tasks
IuliiaHerets opened this issue Nov 6, 2024 · 25 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Design 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

@IuliiaHerets
Copy link

IuliiaHerets commented Nov 6, 2024

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


Version Number: v9.0.58-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: #51588
Issue reported by: Applause Internal Team

Action Performed:

Add copilot

  1. Go to settings > security
  2. Add copilot
  3. Enter an email address
  4. Select any access level
  5. Click on Add copilot

Update copilot role

  1. Go to settings > security
  2. Click a copilot > Change access level
  3. Select the unchecked access level

Expected Result:

The "Verify" button is pinned to the bottom like on the other flows.

Actual Result:

The "Verify" button position is inconsistent for the same step in different flows. In the "Add copilot" flow, the button is displayed below the magic code field. In the "Update copilot role" flow, the button is displayed at the bottom of the right-hand panel.

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?

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6656181_1730872095345.Verify_button.mp4

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @sobitneupane
@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Nov 6, 2024
Copy link

melvin-bot bot commented Nov 6, 2024

Triggered auto assignment to @mallenexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@daledah
Copy link
Contributor

daledah commented Nov 6, 2024

Edited by proposal-police: This proposal was edited at 2024-11-06 16:51:52 UTC.

Proposal

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

The "Verify" button position is inconsistent for the same step in different flows. In the "Add copilot" flow, the button is displayed below the magic code field. In the "Update copilot role" flow, the button is displayed at the bottom of the right-hand panel.

What is the root cause of that problem?

We are using different forms for 2 cases, ValidateCodeForm in this component, ValidateCodeActionModal in this component

And in ValidateCodeForm we are using display flex and justify-content Between to style verify button

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

We should convert this component to use ValidateCodeActionModal to make it consistent with other validated form

<ValidateCodeForm
ref={validateCodeFormRef}
delegate={login}
role={role}
wrapperStyle={safeAreaPaddingBottomStyle}
/>

What alternative solutions did you explore? (Optional)

we can remove styles.justifyContentBetween here

<View style={[styles.flex1, styles.justifyContentBetween, wrapperStyle]}>

@daledah
Copy link
Contributor

daledah commented Nov 6, 2024

updated proposal to add alternative solution and detail root cause

@mallenexpensify mallenexpensify added Weekly KSv2 Daily KSv2 Needs Reproduction Reproducible steps needed and removed Daily KSv2 Weekly KSv2 labels Nov 6, 2024
@MelvinBot
Copy link

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

@mallenexpensify
Copy link
Contributor

I'm out the next week, bumped this to Weekly and added Needs reproduction, doesn't appear to be time sensitive.

@mallenexpensify mallenexpensify added Weekly KSv2 and removed Daily KSv2 labels Nov 6, 2024
@melvin-bot melvin-bot bot added the Overdue label Nov 15, 2024
@mallenexpensify
Copy link
Contributor

Back today, will get to soon.

@melvin-bot melvin-bot bot removed the Overdue label Nov 19, 2024
Copy link

melvin-bot bot commented Nov 20, 2024

@mallenexpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

Copy link

melvin-bot bot commented Nov 21, 2024

Triggered auto assignment to @dubielzyk-expensify (Design), see these Stack Overflow questions for more details.

@mallenexpensify
Copy link
Contributor

@dubielzyk-expensify do you think this is worth fixing? If so, do you know which of the below two is the correct placement for the Verify button? (If we open this up to get fixed I'll see if the contributor can find other instance where the button isn't in the correct spot too)

image SnagitHelper2024 2024-11-20 17 35 12

@dubielzyk-expensify
Copy link
Contributor

Yes worth fixing. We should make sure this button is pinned to the bottom like on the other flows. This came up somewhere recently but I can't find the issue. cc @Expensify/design if they remember.

@shawnborton
Copy link
Contributor

Not sure which issue exactly we addressed it in but agree, we should us the same "Let's make sure it's you" page everywhere, which means the bottom-docked button as you pointed out.

@dannymcclain
Copy link
Contributor

lol yes this has come up a bunch times—basically for all of these magic code screens 😅

I agree with Shawn and Jon!

@mallenexpensify mallenexpensify added the External Added to denote the issue can be worked on by a contributor label Nov 23, 2024
@melvin-bot melvin-bot bot changed the title Web - Copilot - "Verify" button position is inconsistent for the same step in different flow [$250] Web - Copilot - "Verify" button position is inconsistent for the same step in different flow Nov 23, 2024
Copy link

melvin-bot bot commented Nov 23, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 23, 2024
Copy link

melvin-bot bot commented Nov 23, 2024

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Nov 23, 2024
@mallenexpensify
Copy link
Contributor

Thanks design team! Updated the Expected Behaviour in the OP

The "Verify" button is pinned to the bottom like on the other flows.

@sobitneupane when reviewing proposals, I'd love to ask contributors who submit proposals to see if they can find other instances that need updating so we can tackle them in one issue. Thx

Copy link

melvin-bot bot commented Nov 26, 2024

@mallenexpensify, @sobitneupane, @dubielzyk-expensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Nov 26, 2024
@sobitneupane
Copy link
Contributor

Thanks for the proposal @daledah

I could not reproduce the issue. @daledah Were you able to reproduce the issue? Are there any other instances where the button isn't bottom-docked?

@melvin-bot melvin-bot bot removed the Overdue label Nov 26, 2024
@hungdannt
Copy link

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~0199da86ff25cda0b3

Copy link

melvin-bot bot commented Nov 27, 2024

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@hungdannt
Copy link

Proposal

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

The "Verify" button position is inconsistent for the same step in different flows.

  1. In the "Add copilot" flow, the button appears below the magic code field.
  2. In the "Update copilot role" flow, the button is displayed at the bottom of the right-hand panel.

What is the root cause of that problem?

We use different components for 2 flows

  1. "Add copilot" flow : ValidateCodeActionModal (the ValidateCodeForm is wrapped inside this modal).
  2. "Update copilot role" flow: Directly uses ValidateCodeForm.

In the "Update copilot role" flow, the Verify button is correctly styled.
However, at the time this issue was reported, ValidateCodeActionModal in "Add copilot" flow was not styled correctly.
As @sobitneupane said, I spent a lot of time trying to reproduce the error, but couldn't.
Finally, I found it has already been fixed in this commit by adding themeStyles.flex1 😆

<View style={[themeStyles.ph5, themeStyles.mt3, themeStyles.mb7, themeStyles.flex1]}>

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

For better consistency, we should switch to using the ValidateCodeActionModal component, as in the "Add copilot" flow.

I have also checked all other instances, and they display correctly.

What alternative solutions did you explore? (Optional)

@daledah
Copy link
Contributor

daledah commented Nov 28, 2024

Were you able to reproduce the issue? Are there any other instances where the button isn't bottom-docked?

@sobitneupane I can't reproduce this issue too and i can't find any instances where the button isn't bottom-docked

@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@melvin-bot melvin-bot bot added the Overdue label Nov 29, 2024
@sobitneupane
Copy link
Contributor

Issue not reproducible. Good to close.

cc: @mallenexpensify @dubielzyk-expensify

@melvin-bot melvin-bot bot removed the Overdue label Nov 29, 2024
Copy link

melvin-bot bot commented Dec 2, 2024

@mallenexpensify, @sobitneupane, @dubielzyk-expensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@melvin-bot melvin-bot bot added the Overdue label Dec 2, 2024
@mallenexpensify
Copy link
Contributor

Thx, closing due to lack of reproduction

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 Design 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
Development

No branches or pull requests

10 participants