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

[Payment Due] [$500] Web - hovering on the Default currency list of workspace setting leaves the last hovered currency on focus #28041

Closed
6 tasks
kbecciv opened this issue Sep 22, 2023 · 43 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Internal Requires API changes or must be handled by Expensify staff

Comments

@kbecciv
Copy link

kbecciv commented Sep 22, 2023

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 Settings
  2. Click on workspace
  3. Select a workspace
  4. Click on Settings
  5. Click on Default currency
  6. Hover over the currency list and move the cursor off the list

Expected Result:

The focus should be off when the cursor is moved out of the currency list

Actual Result:

The focus stays on the last hovered currency when the cursor is moved out of the currency list

Workaround:

Unknown

Platforms:

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

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • Windows / Chrome
  • MacOS / Desktop

Version Number: 1.3.73.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

2023-09-20.21-56-44.mp4
Recording.4704.mp4

Expensify/Expensify Issue URL:
Issue reported by: @jo-ui
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1695236188869459

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0104c814b5c57b5f48
  • Upwork Job ID: 1705290725148950528
  • Last Price Increase: 2023-10-16
@kbecciv kbecciv added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Sep 22, 2023
@melvin-bot melvin-bot bot changed the title Web - hovering on the Default currency list of workspace setting leaves the last hovered currency on focus [$500] Web - hovering on the Default currency list of workspace setting leaves the last hovered currency on focus Sep 22, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 22, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Sep 22, 2023

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 22, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 22, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • 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
  • 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.
  • 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.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@melvin-bot
Copy link

melvin-bot bot commented Sep 22, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Sep 22, 2023

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

@neg-0
Copy link
Contributor

neg-0 commented Sep 23, 2023

I'm unable to reproduce this when running locally on macOS Chrome and Safari, or in staging on macOS Chrome and Safari. I'll try with Windows and see if I can reproduce it.

@dukenv0307
Copy link
Contributor

dukenv0307 commented Sep 23, 2023

Proposal

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

The focus stays on the last hovered currency when the cursor is moved out of the currency list

What is the root cause of that problem?

That's default behavior for select in Windows. We're using the native select which has different behaviors across platforms and the UI are also different across platform.

This is also inconsistent with how we're selecting currencies in other parts of the app like when requesting money.

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

We should reuse the IOUCurrencySelection page for this instead of using native select (the name of the page should be renamed to something like CurrencySelection. That page is exactly for selecting currency, it's our own implementation that is working well and consistent in all platforms. Also it supports searching for the currency which is very convenient for the user.

The entry point from Workspace settings will look something like this (similar UI to CurrencyPicker)
Screenshot 2023-09-23 at 10 59 54

An alternative (if not a new page) is to build a CurrencyPicker modal, similar to CountryPicker or StatePicker, it will have similar UI to the IOUCurrencySelection page, just that it's a modal only.

What alternative solutions did you explore? (Optional)

NA

@melvin-bot melvin-bot bot added the Overdue label Sep 25, 2023
@burczu
Copy link
Contributor

burczu commented Sep 25, 2023

I'm also unable to reproduce it on macOS with Safari, and as @dukenv0307 pointed out, it's the default behaviour on Windows, so I think we should perhaps re-think if this is a bug or not.

@dukenv0307
Copy link
Contributor

@burczu it's inconsistent across platform and also inconsistent with other currency selection UX in the app itself. What do you think of my suggestion here to standardize it to the IOUCurrencySelection?

@melvin-bot melvin-bot bot added the Overdue label Sep 28, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 29, 2023

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

@burczu
Copy link
Contributor

burczu commented Sep 29, 2023

@dukenv0307 I think making this consistent makes sense, but I think it's better to choose you alternative solution with CurrencyPicker modal instead of creating/using the whole page - what we have here is just a form input, so no need to keep navigation while refreshing the page etc.

That being said, if we agree it is a bug, I think we could choose this proposal to consider.

🎀 👀 🎀 C+ reviewed

@melvin-bot
Copy link

melvin-bot bot commented Sep 29, 2023

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

@melvin-bot melvin-bot bot added the Overdue label Oct 2, 2023
@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 2, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 3, 2023

@burczu, @peterdbarkerUK, @techievivek, @dukenv0307 Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Dec 26, 2023
@techievivek
Copy link
Contributor

Bumped @peterdbarkerUK in DM, we will soon review the compensation status here. Thanks

@melvin-bot melvin-bot bot removed the Overdue label Jan 2, 2024
@peterdbarkerUK
Copy link
Contributor

Hey folks - I'm sorry, I missed this when I got sick right before the holidays. Going to discuss internally.

@peterdbarkerUK
Copy link
Contributor

Hey @dukenv0307 thank for raising this!

I've looked into it and come out with the following timeline, would you agree this is about right?

  • Sep 22, issue posted.
  • Sep 23, solution proposed.
  • Sep 29, solution accepted.
  • Oct 3, work begun on solution, PR resolving issue uncovered, solution placed on hold.
  • Oct 16 PR deployed, issue resolved.

@techievivek
Copy link
Contributor

Not overdue, @dukenv0307 gentle bump ^

@melvin-bot melvin-bot bot removed the Overdue label Jan 15, 2024
@dukenv0307
Copy link
Contributor

@peterdbarkerUK Yeah, I agree with this timeline.

@mountiny
Copy link
Contributor

Unassigned @burczu as no payment is required and they are not c+ anymore

@melvin-bot melvin-bot bot added the Overdue label Jan 24, 2024
@techievivek
Copy link
Contributor

Not overdue, @peterdbarkerUK, can you please review the compensation here? @dukenv0307 agreed with the above timeline #28041 (comment)

@melvin-bot melvin-bot bot removed the Overdue label Jan 29, 2024
@kadiealexander kadiealexander added Bug Something is broken. Auto assigns a BugZero manager. and removed Bug Something is broken. Auto assigns a BugZero manager. labels Feb 1, 2024
Copy link

melvin-bot bot commented Feb 1, 2024

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Feb 1, 2024
@alexpensify
Copy link
Contributor

@dukenv0307 - sorry for the slow action here, I'm catching up and will complete this process on Monday.

I see the timeline was agreed to here: #28041 (comment)

I need to now figure out the payment breakdown and make sure there is an Upwork job for it.

@alexpensify alexpensify changed the title [$500] Web - hovering on the Default currency list of workspace setting leaves the last hovered currency on focus [Payment Due] [$500] Web - hovering on the Default currency list of workspace setting leaves the last hovered currency on focus Feb 2, 2024
@melvin-bot melvin-bot bot added the Overdue label Feb 5, 2024
@techievivek
Copy link
Contributor

@alexpensify, thanks for taking a look into it. To provide some context, @dukenv0307 initially proposed a solution for this GH issue, which was approved by @burczu(Callstack team). However, @dukenv0307 mentioned that a similar bug was being addressed concurrently, so we decided to put this solution on hold. Subsequently, after merging the hold PR, we discovered that the bug was resolved.

As mentioned by @dukenv0307 here, we encountered similar cases in the past, but the contributors were compensated in full. Please let me know if you want any help. Thanks 🙇

@melvin-bot melvin-bot bot removed the Overdue label Feb 5, 2024
@alexpensify
Copy link
Contributor

alexpensify commented Feb 5, 2024

Here is the payment summary:

  • External issue reporter - N/A since the issue was addressed in another GH
  • Contributor that fixed the issue - $500 @dukenv0307 for working on the PR
  • Contributor+ that helped on the issue and/or PR - N/A since this issue was addressed in another GH and the C+ is part of Callstack

Upwork Job: https://www.upwork.com/jobs/~0104c814b5c57b5f48

Notes: Thanks @techievivek for the summary here: #28041 (comment). You helped speed up my process.

@alexpensify
Copy link
Contributor

@dukenv0307 thanks again for your patience-- I've completed the process in Upwork. Closing!

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 Internal Requires API changes or must be handled by Expensify staff
Projects
None yet
Development

No branches or pull requests