[HOLD for payment July 20] Design updates to the Expensify.cash login page

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.49k stars 2.84k forks source link

[HOLD for payment July 20] Design updates to the Expensify.cash login page #2938

Closed stephanieelliott closed 3 years ago

stephanieelliott commented 3 years 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!

Deliverables:

Apply new design and copy updates on the login page at expensify.cash

Fonts and design assets are available in the code repository
Once hired, we will also share the Figma file directly to assist with font sizing
This project has a firm 7-day turnaround time

New (desired) design:

Current (old) design:

Platform:

✔️ Web ✔️iOS ✔️Android ✔️Desktop App ✔️Mobile Web

Version Number: 1.0.45-0

Expensify/Expensify Issue URL: https://github.com/Expensify/Expensify/issues/162030

View all open jobs on Upwork

MelvinBot commented 3 years ago

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

MelvinBot commented 3 years ago

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

stephanieelliott commented 3 years ago

This job has been posted to Upwork: https://www.upwork.com/jobs/~013f470ea624127142

pranshuchittora commented 3 years ago

Proposal 📄

Apply new design and copy updates on the login page at expensify.cash

Fonts and design assets are available in the code repository
Once hired, we will also share the Figma file directly to assist with font sizing
This project has a firm 7-day turnaround time

Approach 👨🏼‍💻

File of concern : /src/pages/signin/LoginForm.js

As we need to introduce changes to the web, we need to consider both the layouts (mobile and desktop) SignInPageLayoutNarrow & SignInPageLayoutWide. Similarly .native.js as well
Styling should be done using the style helper already present in the code styles.p1, ...
~~Images should be optimised hence webp or png will be used (if approved). This will improve the performance and reduce the Time to First Meaningful Paint (FMP).~~
As this is the root/entry point of the app accessibility should be taken care of to improve UX.

Best Practices 💃🏼

No hard-coded string (will be using en.js)
No inline styling
~~Accessibility~~
Follow React Native best practices

Testing Strategy 🧪

Basic render tests

Expected Delivery Time 📦

Approx 1 week

Previous Experience 🙅🏼‍♂️

https://github.com/Expensify/Expensify.cash/issues/2811

pranshuchittora commented 3 years ago

@stephanieelliott looking forward to your feedback on the proposal ☝🏼 Good Day :)

tgolen commented 3 years ago

Hi @pranshuchittora I had a couple of questions/concerns about your proposal.

It doesn't appear from the problem statement that using highly optimized assets is a concern yet. I think it would be best to plan to use whatever is provided and we can come back later and optimize it if we need to.

I don't think accessibility is part of this at all either, so you don't need to worry about that in your proposal.

I think if you remove both of those things, then the rest looks good to me! 👍

stephanieelliott commented 3 years ago

@pranshuchittora I extended the offer to you on Upwork!

pranshuchittora commented 3 years ago

@stephanieelliott can you pls share the Figma files

shawnborton commented 3 years ago

Here is the Figma file: https://www.figma.com/file/E8mgPW9ns9EXr8V2LAKE9B/Sign-Up-Sign-In?node-id=425%3A1016

And here is the image asset exported @2x - ideally it would be displayed at 334px x 551px. expensify-cash-sign-in-screenshot.png.zip

pranshuchittora commented 3 years ago

@tgolen there are few props over here https://github.com/Expensify/Expensify.cash/blob/main/src/pages/signin/SignInPageLayout/SignInPageLayoutNarrow.js#L23:L24

I am not sure how to deal with them because in the new narrow design there is not screenshot (context shouldShowWelcomeScreenshot)

tgolen commented 3 years ago

If those aren't part of the new design, I think you can just remove them. I looked at them, and it doesn't look like those props are passed from anywhere either, so it should be fine to clean those up.

pranshuchittora commented 3 years ago

If those aren't part of the new design, I think you can just remove them. I looked at them, and it doesn't look like those props are passed from anywhere either, so it should be fine to clean those up.

https://github.com/Expensify/Expensify.cash/blob/dc0c8b74ac04e3fd22865a3229743f0d270f7b43/src/pages/signin/SignInPage.js#L88:L89

Will remove that, but not sure about the welcome text one

tgolen commented 3 years ago

Well, they are being passed there as you point out, but SignInPageLayout doesn't pass them to either SignInPageLayoutWide or SignInPageLayoutNarrow, so I think they are totally useless.

tgolen commented 3 years ago

@pranshuchittora Is there an update on this?

MelvinBot commented 3 years ago

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

tgolen commented 3 years ago

Oops, sorry Tom, I didn't mean to assign this to you.

MelvinBot commented 3 years ago

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

tgolen commented 3 years ago

@timszot I am going to be going OOO for the next 10 days, so I reassigned this using the exported chore. Thanks for taking over for me!

stephanieelliott commented 3 years ago

Hey @pranshuchittora, there were changes requested on the PR you submitted. Can you please address these ASAP? Thanks 😃

stephanieelliott commented 3 years ago

Another bump @pranshuchittora, we're nearing a deadline on this - can you please address the requested changes?

pranshuchittora commented 3 years ago

@stephanieelliott there's some issue with the refactored/migrated like opening component TextLink. I tried migrating but facing some issue. It would be great If I get a response on this

https://github.com/Expensify/Expensify.cash/pull/3079#issuecomment-859872305

Can you pls amplify this, so that it gets addressed ASAP

pranshuchittora commented 3 years ago

Blocker due to TermsWithLicenses component Slack thread -> https://expensify.slack.com/archives/C01GTK53T8Q/p1623814030375300

stephanieelliott commented 3 years ago

Add'l changes requested, waiting on them to be addresses and then re-review.

pranshuchittora commented 3 years ago

Add'l changes requested, waiting on them to be addresses and then re-review.

I have updated the PR waiting for review

stephanieelliott commented 3 years ago

More changes applied, PR is awaiting internal review!

Jag96 commented 3 years ago

Reopening to keep track of payment