Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.58k stars 2.92k forks source link

Web - 2FA - "Disable two factor authentication" button text in Spanish is truncated at the end #20785

Closed kbecciv closed 1 year ago

kbecciv commented 1 year ago

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. Change language settings to Spanish
  2. Go to security -> 2FA -> copy / download code -> continue
  3. Download an authenticator app
  4. Enter the auth code generated and complete the 2FA enable steps
  5. On the last page, notice that the text "disable two-factor authentication" is truncated at the end

Expected Result:

Since its an important text it should not be truncated at the end. OR There should be right padding to maintain consistency with the left padding.

Actual Result:

Disable two factor authentication" button text in Spanish is truncated at the end

Workaround:

Unknown

Platforms:

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

Version Number: 1.3.27.6

Reproducible in staging?: yes

Reproducible in production?: n/a

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

Screenshot 2023-06-08 at 3 06 51 PM (1)

https://github.com/Expensify/App/assets/93399543/e3d37323-9a0f-4b57-a270-da417b78c9ff

https://github.com/Expensify/App/assets/93399543/95882ff6-1af7-4973-8147-0716903a93c6

Expensify/Expensify Issue URL:

Issue reported by: @ashimsharma10

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1686216231940599

View all open jobs on GitHub

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

Bug0 Triage Checklist (Main S/O)

dhairyasenjaliya commented 1 year ago

Proposal

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

What is the root cause of that problem?

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

Solution 1

Solution 2

jliexpensify commented 1 year ago

@kbecciv I can't access the Slack thread since it's in a different workspace (the Applause one). Was this posted in the general Expensify-Bugs channel?

Also, running the sentence through Google translate, the Spanish is a bit longer:

image

jliexpensify commented 1 year ago

@dhairyasenjaliya - I'm curious about your proposal: is there any chance that you're able to share a mockup of what it might look like? I ask because I'd imagine we would want to keep everything visually consistent so I'm wondering if your proposal makes the spacing/alignment off (or the words smaller) vs if it's just that the Spanish translation is longer than the English one.

Basically, I'm trying to work out if this is an actual bug/issue or if it's just unfortunately due to the length of the Spanish translation.

hungvu193 commented 1 year ago

Proposal

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

Web - 2FA - "Disable two factor authentication" button text in Spanish is truncated at the end

What is the root cause of that problem?

The text in Spanish of menuItems is too long, so it's truncated at the end. https://github.com/Expensify/App/blob/23ca9a7907738e0bd0e0ad53957946097b090b94/src/pages/settings/Security/TwoFactorAuth/IsEnabledPage.js#L29-L38

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

Because the close icon take a lot of space and I think we're doing a danger action about app's security, I think we can change the menuItems to Button with danger style. So we should remove the menuItems and add the Button inside Section, below the Text.

            <Section
                title={props.translate('twoFactorAuth.twoFactorAuthEnabled')}
                icon={Illustrations.ShieldYellow}
                containerStyles={[styles.twoFactorAuthSection]}
            >
                <View style={styles.mv3}>
                    <Text style={styles.textLabel}>{props.translate('twoFactorAuth.whatIsTwoFactorAuth')}</Text>
                    <Button
                        style={styles.mt5}
                        success
                        pressOnEnter
                        text={props.translate('twoFactorAuth.disableTwoFactorAuth')}
                        onPress={() => setIsConfirmModalVisible(true)}
                        danger
                    />
                </View>
            </Section>

Here's how it looks:

Screenshot 2023-06-15 at 10 17 00

If we still want to keep the menuItems we can just remove the icon and add the styles danger to menuItems.

What alternative solutions did you explore? (Optional)

The simple way, since we already explain the meaning of 2FA at the start of the page:

Two-factor authentication (2FA) helps keep your account safe. When logging in, you’ll need to enter a code generated by your preferred authenticator app.

We can use that word here

Screenshot 2023-06-15 at 10 23 58
dhairyasenjaliya commented 1 year ago

hey @jliexpensify basically once we have the long text and we do not have any right icon at that time we are not adding any padding to the right which makes the text display without padding this is inconsistency with the left padding below I have attached the screenshot of current code in which we have 0px padding from the right and on fix we are adding 8px padding which is consistency with left padding

Current Code

broken

Fix with padding adjustment

fix
jliexpensify commented 1 year ago

Hmm ok, I'm still not convinced that this is an actual issue - I think it's to do with the length of the translation. I've asked internally what we should do and will keep this GH posted, thanks!

ashimsharma10 commented 1 year ago

@jliexpensify as per your above query, it was posted in #expensify-bugs . Find it here . https://expensify.slack.com/archives/C049HHMV9SM/p1686216231940599.

Also, we have a lot of places in App, where exact Spanish translation isn't used. I think we have a native Spanish speakers' team doing that. And might not be the issue. However, for me, truncation should be padded well.

Found a similar bug related to turncation here https://github.com/Expensify/App/issues/16524?fbclid=IwAR03AI94y-WcZp9VKGmIon3Jn_3LOJVhFmRJ05cDuGUN4t6wGFuUrOa7mMo

jliexpensify commented 1 year ago

Thanks for sharing the link @ashimsharma10 , I've updated the GH.

Funnily enough, I was assigned that issue you linked. That one was due to an actual regression and was missing the ... when the text was truncated.

Let me check internally what we want to do - I could very likely be wrong here. I appreciate everyone's input in the meantime!

jliexpensify commented 1 year ago

Hi everyone, just circling back to this issue: I have raised it and shared the proposals internally, and the decision was made to do nothing for now. internally, one of our Spanish-speaking engineers said that the message is still understandable even if it's truncated so there isn't a need to do anything here.

Thanks for everyone's proposals!