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

Login page welcome text style differs between platforms (Inconsistent) #4729

Closed
4 tasks done
Santhosh-Sellavel opened this issue Aug 18, 2021 · 34 comments
Closed
4 tasks done
Assignees
Labels
External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@Santhosh-Sellavel
Copy link
Collaborator

Santhosh-Sellavel commented Aug 18, 2021

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. Logout If already logged in, log out.
  2. See the welcome text

Expected Result:

The same style everywhere!

Actual Result:

It's bolded in Android, not bolded everywhere else!

Note: Ignore screens have some error messages.

Android

Screenshot_1629301486

iOS

Simulator Screen Shot - iPhone 12 - 2021-08-18 at 21 12 24

mWeb

Simulator Screen Shot - iPhone 12 - 2021-08-18 at 21 11 16

Desktop

Screenshot 2021-08-18 at 9 09 27 PM

Web

Screenshot 2021-08-18 at 9 08 27 PM

Workaround:

Can the user still use Expensify without this being fixed? Yes

Platform:

Where is this issue occurring?
Confused, Let's say all for now.

  • Web
  • iOS
  • Desktop App
  • Mobile Web

Version Number:
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:

View all open jobs on GitHub

Suggestion

I don't know what's expected or which is correct. But style should be the same across all platforms!

@Santhosh-Sellavel Santhosh-Sellavel added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels Aug 18, 2021
@MelvinBot
Copy link

Triggered auto assignment to @sakluger (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@MelvinBot MelvinBot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Aug 18, 2021
@Santhosh-Sellavel
Copy link
Collaborator Author

Proposal

I do see this happens because of the following reasons.
The style we use for welcome text is
Screenshot 2021-08-18 at 9 56 18 PM

In the h3 style for fontFamily.GTA_BOLD, we have written a separate export for bold style alone.
Screenshot 2021-08-18 at 10 01 06 PM

bold contains following files, style for android & index.js for all the other platforms
Screenshot 2021-08-18 at 9 55 10 PM

index.android.js
Screenshot 2021-08-18 at 9 55 16 PM

index.js
Screenshot 2021-08-18 at 9 55 25 PM

So this is the reason we have different styles between platforms. We could modify the same to fix the issue! But first, we want to figure out what's intended.

@shawnborton
Copy link
Contributor

I think we want the text to appear bold on all platforms. Does this mean all bold text is broken on Android?

@Santhosh-Sellavel
Copy link
Collaborator Author

Santhosh-Sellavel commented Aug 18, 2021

Yes, it looks like @shawnborton!

@Santhosh-Sellavel
Copy link
Collaborator Author

Santhosh-Sellavel commented Aug 18, 2021

Do you expect GTAmericaExp-Regular for all bold styles? @shawnborton

@shawnborton
Copy link
Contributor

I would expect GTAmericaExpBold for bold.

@Santhosh-Sellavel
Copy link
Collaborator Author

GTAmericaExp-Bold does not work well for web/desktop!

@shawnborton
Copy link
Contributor

I'm not sure what to tell you then - perhaps @roryabraham or @marcaaron might have an idea as I think one of them might have worked on this?

@Santhosh-Sellavel
Copy link
Collaborator Author

Okay, @shawnborton it does not look like it is completely messed up, I will investigate further for other pages!

@Santhosh-Sellavel
Copy link
Collaborator Author

It's not working on login pages alone as expected!

@Santhosh-Sellavel
Copy link
Collaborator Author

Santhosh-Sellavel commented Aug 21, 2021

After further investigation, I found the reasons.

For all other styles which use fontFamily.GTA_BOLD as fontFamily in styles.js pass along fontWeight: fontWeightBold so the text looks bold in all other pages as expected.
In below lines h1 style has property fontWeight: fontWeightBold, but for h3 and h4 we missed it out.

App/src/styles/styles.js

Lines 53 to 68 in bdcafa3

h1: {
color: themeColors.heading,
fontFamily: fontFamily.GTA_BOLD,
fontSize: variables.fontSizeh1,
fontWeight: fontWeightBold,
},
h3: {
fontFamily: fontFamily.GTA_BOLD,
fontSize: variables.fontSizeNormal,
},
h4: {
fontFamily: fontFamily.GTA_BOLD,
fontSize: variables.fontSizeLabel,
},

So It's not bold in login form pages alone. In the below screens, h3 style is used for the Welcome back to the New Expensify! Please enter your password. & h4 style is used for Forget? Look at the differences. It's all bolded in android and missed on the web.

Web

Screenshot 2021-08-21 at 5 05 51 PM

Android

Screenshot_1629546813

cc: @shawnborton @roryabraham

De we need the messages bolded here or not?
If so we need to add fontWeight: fontWeightBold to h3 & h4 styles
if not, then we need update fontFamily: fontFamily.GTA in h3 & h4 styles

Post your thoughts on this thanks!

@shawnborton
Copy link
Contributor

Okay so sounds like we just need to add fontWeightBold to the h3/h4 styles then, as we do want those headings to appear bold. Thanks for investigating!

@roryabraham
Copy link
Contributor

This looks good and a cursory search revealed that it doesn't appear to be a duplicate. Going to tag this as external so that we can get @Santhosh-Sellavel hired to make this change.

@roryabraham roryabraham added the External Added to denote the issue can be worked on by a contributor label Aug 23, 2021
@MelvinBot
Copy link

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

@roryabraham
Copy link
Contributor

@marklouisdeshaun Let's get an Upwork job created for this, then hire @Santhosh-Sellavel.

@Santhosh-Sellavel Feel free to submit a PR once you've been hired on Upwork.

@Santhosh-Sellavel
Copy link
Collaborator Author

Sure @roryabraham, thanks!

@marklouisdeshaun
Copy link
Contributor

Job created in Upwork!

Internal post: https://www.upwork.com/ab/applicants/1430289271743135744/job-details
External post: https://www.upwork.com/jobs/~019e6eda1b98605ada

@MelvinBot MelvinBot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 24, 2021
@muhammadtaha1998
Copy link

I think its just simple style issue. Need to change it from GTAmericaExp-Regular to GTAmericaExp-Bold

@Santhosh-Sellavel
Copy link
Collaborator Author

@roryabraham @marklouisdeshaun I applied for the job! Shall I wait for PR until hired, can I start my PR work?

@roryabraham roryabraham removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 25, 2021
@roryabraham
Copy link
Contributor

@Santhosh-Sellavel Up to you! Feel free to start the PR work if you want to get a head-start.

@roryabraham
Copy link
Contributor

@Santhosh-Sellavel I went ahead and hired you on Upwork

@Santhosh-Sellavel
Copy link
Collaborator Author

Santhosh-Sellavel commented Aug 25, 2021

@shawnborton @roryabraham
Simulator Screen Shot - iPhone 12 - 2021-08-26 at 00 52 45

  1. The highlighted area is not aligned with other views above in mobile iOS.
  2. Go back is not bolded what can we do here? We need to unify

@shawnborton
Copy link
Contributor

"Go back" and "forgot" should not be bolded.

The highlighted area is not aligned with other views above in mobile iOS.

What do you mean by that?

@Santhosh-Sellavel
Copy link
Collaborator Author

Santhosh-Sellavel commented Aug 25, 2021

This from web,
Screenshot 2021-08-26 at 12 46 43 AM

this is iOS
Simulator Screen Shot - iPhone 12 - 2021-08-26 at 00 52 22

By logging in, you agree
that area is not aligned with input/buttons
@shawnborton

@shawnborton
Copy link
Contributor

Got it, I agree with that and it actually looks like that's even more broken - the small print stuff should be at the bottom:

image

@Santhosh-Sellavel
Copy link
Collaborator Author

Even for mWeb & mobile? it should be on the bottom? @shawnborton

@Santhosh-Sellavel
Copy link
Collaborator Author

Santhosh-Sellavel commented Aug 25, 2021

There is a different layout style intentionally for mobile/smallscreen width kindy confirm me in thanks!
Screenshot 2021-08-26 at 2 46 16 AM
cc: @shawnborton @roryabraham

@Santhosh-Sellavel
Copy link
Collaborator Author

@roryabraham Changes discussed earlier, Forget? style is updated in PR & ready for review!

@shawnborton
Copy link
Contributor

There is a different layout style intentionally for mobile/smallscreen width kindy confirm me in thanks!

The sign in screen on wider devices has the blue area on the right, otherwise the gray area that has the logo + form + footer should look identical on all platforms.

@Santhosh-Sellavel
Copy link
Collaborator Author

Got it, I agree with that and it actually looks like that's even more broken - the small print stuff should be at the bottom:

image

cc: @roryabraham @shawnborton I'll take this as a separate issue, do you feel otherwise let me know!

@marklouisdeshaun marklouisdeshaun removed their assignment Aug 26, 2021
@marklouisdeshaun marklouisdeshaun added External Added to denote the issue can be worked on by a contributor and removed External Added to denote the issue can be worked on by a contributor labels Aug 26, 2021
@MelvinBot
Copy link

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

@marklouisdeshaun
Copy link
Contributor

I've re-assigned as I'll be OOO so that a member of the team can stay on top of this. Thanks!

@roryabraham
Copy link
Contributor

Looks like the PR was merged, making this a weekly while we wait for it to be deployed to prod.

@mallenexpensify
Copy link
Contributor

Apologies for the delay in payment @Santhosh-Sellavel , I just paid in Upwork now and added the bonus for reporting the issue. In the very near future, if not now, there will be an updated comment about when to pay issues and the title will be updated, so this shouldn't continue to happen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

9 participants