[HOLD for payment 2022-12-20] [$1000] A visual indicator like a tooltip should be present for currency selector reported by @daraksha-dk

[kavimuru]

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

---

Problem:

User doesn't know if he can also change the currency here by clicking on the symbol

Solution:

A visual indicator like a tooltip should be present for currency selector like we have for almost every action on the app except for this one.

Platform:

Where is this issue occurring?

• Web
• Desktop App

Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Expensify/Expensify Issue URL:
Issue reported by: @daraksha-dk
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1666716588685909

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~018d2593b6d75e5a34
• Upwork Job ID: 1600875211272773632

[daraksha-dk]

Proposal

The CurrencySymbolButton can be wrapped with the Tooltip

App/src/components/TextInputWithCurrencySymbol.js

Line 55 in cb93661

<CurrencySymbolButton

or it can be done at

App/src/components/CurrencySymbolButton.js

Lines 17 to 19 in cb93661

	<TouchableOpacity onPress={props.onCurrencyButtonPress}>
	<Text style={styles.iouAmountText}>{props.currencySymbol}</Text>
	</TouchableOpacity>

+        <Tooltip text="currency">
            <TouchableOpacity onPress={props.onCurrencyButtonPress}>
                <Text style={styles.iouAmountText}>{props.currencySymbol}</Text>
            </TouchableOpacity>
+        </Tooltip>

we will need to add the text at en.js and es.js we will need to use this.props.translate('') and use the key we will add and we need to use withLocalize to get this.props.translate

[rushatgabhane]

i don't think this problem statement was agreed by anyone. is it even a real problem?

im not a fan of the solution either, it doesn't fix for all platforms.

[daraksha-dk]

@kavimuru I think you forgot to add AutoAssignerTriage tag here.

Got the approval here: https://expensify.slack.com/archives/C01GTK53T8Q/p1668006728010219?thread_ts=1666716588.685909&cid=C01GTK53T8Q

[rushatgabhane]

@daraksha-dk New feature requests aren't a priority and won't be fixed until all/most bugs have been squashed

[melvin-bot]

Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[marcaaron]

Hmm actually - I think this would be good to fix. It does feel like a bug kind of because the currency selector is pretty hard to discover and the change is very low in complexity.

[melvin-bot]

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

[bfitzexpensify]

I agree @marcaaron

Bug0 Triage Checklist

Note: see this SO for more information.

• The "bug" is actually a bug - yes, given
• The bug is not a duplicate report of an existing GH.
  • If it is, close the GH and add any novel details to the original GH instead
• The bug is reproducible, following the reproduction steps.
• The GH template is filled out as fully as possible
  • The GH body and title are clear (ie. could an external contributor understand it and work on it?)
• The GH was created by an Expensify employee or a QA tester
• Decide if the GH should be resolved by an External contributor or Internal engineer, add the appropriate label

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[jatinsonijs]

I think Tooltip is just for web. same concern is also apply on another platforms. And current tooltip still depends on user action (User have to move cursor above the symbol to know they can change currency by that symbol).

[MitchExpensify]

@dangrous this is one of the oldest issues in the /App repo. To help us clear out the large backlog of bugs, can you:

• Decide whether any proposals currently meet our guidelines and can be approved as-is
• For any that can't, please take this issue internal and treat it as one of your highest priorities
• If you have any questions, don't hesitate to start a discussion in #bug-zero

[dangrous]

Since this is sort of a cross between a bug and a feature request, I think we're okay with moving forward with @daraksha-dk's solution for web, and we can circle back around to mobile as a future development. I'll assign you now!

[melvin-bot]

📣 @daraksha-dk You have been assigned to this job by @dangrous!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[daraksha-dk]

PR is ready. I have added the Moneda as translation of currency. I referenced it from the line below. Please verify if it's correct.

App/src/languages/es.js

Line 148 in 5ab92f5

selectCurrency: 'Selecciona una moneda',

[shawnborton]

Do we want the tooltip to say “Change currency”? Not sure what the Spanish translation is for that, maybe “Cambiar la moneda” but let’s get confirmation on that first

…

On Fri, Dec 9, 2022 at 8:52 AM Daraksha ***@***.***> wrote: PR is ready. I have added the Moneda as translation of currency. I referenced it from the line below. Please verify if it's correct. https://github.com/Expensify/App/blob/5ab92f54240381b7e2ebe78af0129d429c4d6b95/src/languages/es.js#L148 — Reply to this email directly, view it on GitHub <#12555 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AARWH5UN3CNLAJFQF3VFNXDWMNPUVANCNFSM6AAAAAAR2PCWGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[mananjadhav]

I am +1 for Change currency. @dangrous What do you think?

[dangrous]

Do we just want to use the same selectCurrency line? Since it's already translated? I feel like that has the same meaning as Change currency

[mananjadhav]

That works too.

[shawnborton]

Great idea!

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.38-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Add tooltip for currency picker button #13476

If no regressions arise, payment will be issued on 2022-12-20. 🎊

After the hold period, please check if any of the following need payment for this issue, and if so check them off after paying:

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@mananjadhav / @dangrous] The PR that introduced the bug has been identified. Link to the PR: [Fix] Localize currency symbol #8299
• [@mananjadhav / @dangrous] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: [Fix] Localize currency symbol #8299 (comment)
• [@mananjadhav / @dangrous] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
• [@bfitzexpensify] A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here: https://github.com/Expensify/Expensify/issues/251595

[dangrous]

@mananjadhav This wasn't exactly a bug, and certainly not a regression, is that right? The tooltip never existed? If not I guess we can link to the original PR that created the functionality and did not add the tool tip...

[mananjadhav]

This wasn't exactly a bug, and certainly not a regression, is that right? The tooltip never existed?

That's right. I think this PR added the component but I am guessing the dropdown existed even before creating the component

[dangrous]

Okay, linked in the checklist and added in a comment! I'm making an executive decision that we don't need a discussion in #expensify-bugs for this because it wasn't a bug, just a miss on the initial conception of the feature. It'll be good in the future to make sure users have an easy way of finding out about "hidden" features like this but I don't think that would be an effective conversation to have in the bugs channel, more something that should already be thought about.

@bfitzexpensify I think maybe we make a regression test to just check if the tooltip is there on web and desktop? And then we can close out.

[mananjadhav]

@bfitzexpensify this should be eligible for the timeline bonus.

[bfitzexpensify]

Yup! All upwork contracts paid and ended. Regression test suggested update posted here. Once that's done I'll get that wrapped up and close this out.

[bfitzexpensify]

OK, regression test updated. We're all done here.