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

[HOLD][$250] Android - Attachment - The error notification is very close to the boarder of the phone #10031

Closed
kbecciv opened this issue Jul 20, 2022 · 18 comments

Comments

@kbecciv
Copy link

kbecciv commented Jul 20, 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. Launch the app
  2. Log in with any account
  3. Tap on any user
  4. Tap on Plus button - Add attachment
  5. Select option from Choose from gallery
  6. Tap on any 3 MB mp4 format
  7. Tap Send

Expected Result:

The error notification is not very close to the boarder of the phone

Actual Result:

The error notification is very close to the boarder of the phone

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • Web
  • iOS - there is no error message
  • Android
  • Desktop App
  • Mobile Web

Version Number: 1.1.85.5

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

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 Jul 20, 2022

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

@tylerkaraszewski tylerkaraszewski added the External Added to denote the issue can be worked on by a contributor label Jul 21, 2022
@tylerkaraszewski tylerkaraszewski removed their assignment Jul 21, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jul 21, 2022

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

@tylerkaraszewski
Copy link
Contributor

Seems reasonable to work on.

@thesahindia
Copy link
Member

Proposal

I tested this and the text also goes out of the screen.
Adding flex: 1 at inlineSystemMessage will fix that and for some spacing near the border we can pass styles.pr2 here

<View style={[styles.flexRow, styles.alignItemsCenter]}>

@melvin-bot melvin-bot bot added the Overdue label Jul 23, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jul 25, 2022

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

@adelekennedy
Copy link

internal
external

@melvin-bot melvin-bot bot removed the Overdue label Jul 25, 2022
@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Jul 25, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jul 25, 2022

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (Exported)

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 25, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jul 25, 2022

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

@melvin-bot melvin-bot bot changed the title Android - Attachment - The error notification is very close to the boarder of the phone [$250] Android - Attachment - The error notification is very close to the boarder of the phone Jul 25, 2022
@vladnyk
Copy link
Contributor

vladnyk commented Jul 25, 2022

Proposal

<Text style={[styles.inlineSystemMessage]}>{props.message}</Text>

update Text style by adding flex:1, replacing marginLeft: 6 with marginHorizontal: 6 in styles.js
inlineSystemMessage: {

    inlineSystemMessage: {
        flex: 1,
        color: themeColors.textSupporting,
        fontSize: variables.fontSizeLabel,
        fontFamily: fontFamily.GTA,
        marginHorizontal: 6,
    },

style

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jul 25, 2022
@Santhosh-Sellavel
Copy link
Collaborator

@chiragsalian

🎀 👀 🎀 C+ Reviewed

There are two proposals here first one & second one

Both are good & merely similar, the only difference is 1'st proposal suggests pr2 i.e 8 padding-right and the second proposal suggests marginHorizontal: 6 instead. I prefer 1'st proposal but I'll let you decide to take the final call.

Let's get @Expensify/design team's confirmation about the expected padding on the right side.

Thanks!

@shawnborton shawnborton self-assigned this Jul 26, 2022
@shawnborton
Copy link
Contributor

I actually wonder if we want to hold off on fixing this one until some of our offline pattern styles are deployed. Basically this error style should soon be updated with just a red dot instead of the !

@iwiznia what are your thoughts there?

But if we do decide just to fix this quickly, I would think the padding on the right of the error text should be 20px to match the text above and below it.

@iwiznia
Copy link
Contributor

iwiznia commented Jul 26, 2022

Yeah, this will be refactored soon too, I think we should hold.

@chiragsalian
Copy link
Contributor

Sounds good, demoting, placing HOLD and removing external and help wanted label for now.

@chiragsalian chiragsalian added Monthly KSv2 and removed External Added to denote the issue can be worked on by a contributor Daily KSv2 Help Wanted Apply this label when an issue is open to proposals by contributors labels Jul 27, 2022
@chiragsalian chiragsalian changed the title [$250] Android - Attachment - The error notification is very close to the boarder of the phone [HOLD][$250] Android - Attachment - The error notification is very close to the boarder of the phone Jul 27, 2022
@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@mvtglobally
Copy link

Issue not reproducible during KI retests. (Second week)

@thesahindia
Copy link
Member

thesahindia commented Aug 19, 2022

We can close this. Now we are using popover to display the error.

@mvtglobally
Copy link

Issue not reproducible during KI retests. (Third week)
@chiragsalian @adelekennedy are we ok to close this?

@adelekennedy
Copy link

I think we're good to close!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants