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

[EP-2362] Implement left panel/header from design #1148

Merged
merged 12 commits into from
Feb 13, 2025

Conversation

canac
Copy link
Contributor

@canac canac commented Feb 7, 2025

Description

  • Implement the left panel from the new design for the sign in and sign up modals. On small screens, the left panel becomes a header.
  • Also, I did some styling cleanup of the modals inside the register account modal.

It is not reflected in the Figma design, but when a user is logged in or identified, we show a minimal version of the modals that doesn't include the left panel or header.

I've tested this thoroughly, but there are a lot of combinations of states, so it would be great to get another pair of eyes on it in case I missed something.

Testing

  • Completely log yourself out and test the sign in and sign up modals with a large and small screen width.
  • Delete the give-session cookie to test the minimal mode of the sign in and sign up modals with a large and small screen width. (Or set this.newUser = false in registerAccountModal.component.js)
  • In the subscription callback in $onInit in registerAccountModal.component.js, add return this.stateChanged('contact-info') to test the contact info modal with a large and small screen.
  • Override the registration-state locally to test the user match modal.

@canac canac requested review from dr-bizz and rguinee February 7, 2025 23:05
@canac canac self-assigned this Feb 10, 2025
Copy link
Contributor

@dr-bizz dr-bizz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking amazing! Thank you for making this so good. I just had a few comments.

The "Powered by Okta" text is more faded out on the design.
Screenshot 2025-02-10 at 2 52 56 PM

During responsive testing, when I get to 380 px (some phones), the background image doesn't go all the way down to the button of the div and the text prob needs to get smaller.

You could fix this by changing the image to a background image.

    background: url("/assets/img/register-account-header.jpeg");
    background-position-x: 0%;
    background-position-y: 0%;
    background-size: auto;
    background-size: cover;
    background-position: center;
  }

Uploading Screenshot 2025-02-10 at 2.52.10 PM.png…

webpack.config.js Show resolved Hide resolved
src/common/components/signInForm/signInForm.component.js Outdated Show resolved Hide resolved
@canac
Copy link
Contributor Author

canac commented Feb 10, 2025

The "Powered by Okta" text is more faded out on the design.

@dr-bizz I mentioned to Ryan that the contrast didn't pass WCAG guidelines, and he updated it in one of the screens but not all of them.

@canac
Copy link
Contributor Author

canac commented Feb 10, 2025

During responsive testing, when I get to 380 px (some phones), the background image doesn't go all the way down to the button of the div and the text prob needs to get smaller.
You could fix this by changing the image to a background image.

@dr-bizz I opted to not use a background image because the layout was easier to implement if the image takes up space in the desktop view. I adjusted the styles to fill the whole vertical area even at very small screen widths.

@canac canac requested a review from dr-bizz February 10, 2025 21:05
@rguinee
Copy link
Contributor

rguinee commented Feb 10, 2025 via email

Copy link
Contributor

@dr-bizz dr-bizz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for those changes! The code is looking great!

Copy link
Contributor

@rguinee rguinee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

One thing that may not be an issue when it's merged with the register modal changes.

Background color issue when modal height expands from initial height.

background color

src/assets/scss/_modal.scss Show resolved Hide resolved
@canac canac force-pushed the 2362-okta-modal-left-panel branch from 41740ff to dacddd8 Compare February 13, 2025 21:19
@canac canac merged commit dbb0ff9 into 2362-okta Feb 13, 2025
1 check passed
@canac canac deleted the 2362-okta-modal-left-panel branch February 13, 2025 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants