feat: Auth form visual enhancements (master)

[Lunyachek]

Description

Cosmetic improvements for the login/registration page. Changes in React are related to the situation when third-party login providers is disabled, but when switching between Login and Registration tabs, we still see a preloader that is not necessary in this situation.

2023-03-01.16.08.24.mov

And sass changes are related to the suggested username field, which is too short and can be extended to match the neighboring inputs.

After making our changes, we no longer see preloaders in cases where third-party login providers are turned off.

2023-03-01.16.47.42.mov

And suggested username field looks pretty

[openedx-webhooks]

Thanks for the pull request, @Lunyachek! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

• supporting documentation
• Open edX discussion forum threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.04%. Comparing base (90ae870) to head (f66d610).
Report is 7 commits behind head on master.

❗ Current head f66d610 differs from pull request most recent head 92fcc31. Consider uploading reports for the commit 92fcc31 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #767      +/-   ##
==========================================
- Coverage   87.71%   87.04%   -0.68%     
==========================================
  Files         124      124              
  Lines        2288     2284       -4     
  Branches      636      639       +3     
==========================================
- Hits         2007     1988      -19     
- Misses        272      287      +15     
  Partials        9        9              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[syedsajjadkazmii]

Overall the layout is changed for me, I can see in your screenshots that it's fine but the left panel width is decreased for me. Although it's working fine on the master branch. Could you please also look into this?

[mphilbrick211]

Overall the layout is changed for me, I can see in your screenshots that it's fine but the left panel width is decreased for me. Although it's working fine on the master branch. Could you please also look into this?

Hi @Lunyachek - friendly ping on this :)

[syedsajjadkazmii]

One more thing that we noticed (thanks to @zainab-amir )

If we use this approach for hiding the social auth skeleton when they are inactive, then it will also hide the skeleton when social auth is active. Because we get providers in MFE_context api response and based on the response we render these buttons if providers are available.

[mphilbrick211]

Hi @Lunyachek! Looks like some tests need to be re-run and branch conflicts need to be resolved.

@zainab-amir @syedsajjadkazmii - Product has completed their review. Once the conflicts and tests are all set, please review and merge if all looks good. Thanks!

[zainab-amir]

@mphilbrick211 the changes will introduce a bug as mentioned by Sajjad. Those need to be resolved first.

[mphilbrick211]

Hi @syedsajjadkazmii and @zainab-amir - any update on this?

[syedsajjadkazmii]

Hi @syedsajjadkazmii and @zainab-amir - any update on this?

Hi @mphilbrick211, we are waiting on the author to resolve the suggestions before we proceed with this.

[mphilbrick211]

Hi @syedsajjadkazmii and @zainab-amir - any update on this?

Hi @mphilbrick211, we are waiting on the author to resolve the suggestions before we proceed with this.

@Lunyachek - flagging this for you :)

[mphilbrick211]

Hi @Lunyachek! Just checking to see if you're able to address the items above? Thanks!

[mphilbrick211]

Hi @Lunyachek! Just checking to see if you're able to address the items above? Thanks!

Hi @Lunyachek - are you planning to pursue this pull request? Please let us know if you have any questions. Thanks!

[Lunyachek]

Overall the layout is changed for me, I can see in your screenshots that it's fine but the left panel width is decreased for me. Although it's working fine on the master branch. Could you please also look into this?

I removed changes in css related to the suggested username field because of fresh master branch already includes enhancements for this field. And left bar, shown on this screenshot, increased to it normal size

[mphilbrick211]

Thanks, @Lunyachek! It looks like there's a failing test - would you mind taking a look?

[syedsajjadkazmii]

One more thing that we noticed (thanks to @zainab-amir )

If we use this approach for hiding the social auth skeleton when they are inactive, then it will also hide the skeleton when social auth is active. Because we get providers in MFE_context api response and based on the response we render these buttons if providers are available.

Hi @Lunyachek, could you please consider this scenario too?

[mphilbrick211]

One more thing that we noticed (thanks to @zainab-amir )
If we use this approach for hiding the social auth skeleton when they are inactive, then it will also hide the skeleton when social auth is active. Because we get providers in MFE_context api response and based on the response we render these buttons if providers are available.

Hi @Lunyachek, could you please consider this scenario too?

Hi @Lunyachek! Just checking to see if you plan to pursue this PR?

[Lunyachek]

One more thing that we noticed (thanks to @zainab-amir )

If we use this approach for hiding the social auth skeleton when they are inactive, then it will also hide the skeleton when social auth is active. Because we get providers in MFE_context api response and based on the response we render these buttons if providers are available.

When social auth is active - skeleton will not be hidden. I recorded a video with local check (sorry for quality, I need to made it less than 10mb)

First part - no providers and no skeleton, second part - provider is enabled and skeleton is visible

2023-10-21.14.09.23-2.mov

[mphilbrick211]

Hi @Lunyachek! Would you mind rebasing to see if that fixes the codecov failure?

Also, @zainab-amir just flagging this for review, along with the backport.

[syedsajjadkazmii]

Hi @Lunyachek, you are right. The loader is not hidden when social auth is enabled.

But could you please do the following changes?

I agree, changed to isThirdPartyAuthActive

please change all ThirdPartyAuthPreloader to isThirdPartyAuthActive.

also i think you can use isThirdPartyAuthActive variable instead of condition in the following places
login page component and
third party auth component

And please rebase the branch :)

[mphilbrick211]

Hi @Lunyachek, you are right. The loader is not hidden when social auth is enabled.

But could you please do the following changes?

I agree, changed to isThirdPartyAuthActive

please change all ThirdPartyAuthPreloader to isThirdPartyAuthActive.

also i think you can use isThirdPartyAuthActive variable instead of condition in the following places login page component and third party auth component

And please rebase the branch :)

Hi @Lunyachek! Flagging this for you.

[dyudyunov]

@mphilbrick211 hi

I rebased the branch, now we need someone to trigger workflows

@Lunyachek please check recent comments:

Hi @Lunyachek, you are right. The loader is not hidden when social auth is enabled.
But could you please do the following changes?
I agree, changed to isThirdPartyAuthActive
please change all ThirdPartyAuthPreloader to isThirdPartyAuthActive.
also i think you can use isThirdPartyAuthActive variable instead of condition in the following places
login page component and
third party auth component

[mphilbrick211]

@Lunyachek @dyudyunov - tests have been re-run. Looks like there's still a failing check.

[mphilbrick211]

@Lunyachek @dyudyunov - tests have been re-run. Looks like there's still a failing check.

Hi @Lunyachek and @dyudyunov - it would be great to get this reviewed soon. Flagging the failing check and the branch conflicts that have popped up.

[dyudyunov]

@mphilbrick211 thanks for the ping!
We'll try to work on it ASAP

Note: codecov (failed check) is pointing to lines where no changes were made. The rebasing might fix it

[Lunyachek]

I rebased branch and moved changes to the ThirdPartyAuth.jsx

Also made PR to quince branch - #1225

[mphilbrick211]

Hi @openedx/2u-vanguards! This is ready for review, along with backports #766 and #1225

[openedx-webhooks]

@Lunyachek 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.