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

style(settings): General style updates #18020

Merged
merged 1 commit into from
Nov 15, 2024
Merged

style(settings): General style updates #18020

merged 1 commit into from
Nov 15, 2024

Conversation

vpomerleau
Copy link
Contributor

@vpomerleau vpomerleau commented Nov 14, 2024

Because

  • Switching design to text-start default instead of text-center
  • Some images were cut off on mobile

This pull request

  • Update style for AppLayout
  • Update image styling
  • Various tweaks to accommodate change to text-start
  • Set size for Mozilla logo in Storybook sidebar
  • Replace placeholder image used in storybooks (placekitten server no longer seems to work)
  • Adjust padding for data block, third party auth buttons
  • Fix error banner shown instead of success banner on inline 2FA setup

Issue that this pull request solves

Closes: #FXA-10613, FXA-10741

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).

Screenshots (Optional)

Start align and image fix on mobile:

image

Signin (with start text align) - opted to leave email centered with avatar

image

Signup (sync) with text-start

image

Other information (Optional)

Opted to keep content centered on a few select screens:

Legal:
image

Confirmation pages that only contained a heading, centered image and one-liner text or CTA. E.g.,

image

@vpomerleau vpomerleau requested a review from a team as a code owner November 14, 2024 19:00
@vpomerleau vpomerleau force-pushed the FXA-10613 branch 3 times, most recently from 99b0f3c to d69e5dd Compare November 14, 2024 19:31
Because:

* Switching design to text-start default instead of text-center
* Some images were cut off on mobile

This commit:

* Update style for AppLayout
* Update image styling
* Various tweaks to accomodate change to text-start
* Set size for Mozilla logo in Storybook sidebar
* Replace placeholder image used in storybooks
* Increase spacing below primary buttons

Closes #FXA-10613
Copy link
Contributor

@vbudhram vbudhram left a comment

Choose a reason for hiding this comment

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

@vpomerleau Thanks! Storybook views LGTM 👍🏽

Copy link
Contributor

@LZoog LZoog left a comment

Choose a reason for hiding this comment

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

I didn't check the updated changes but quick glance through the code changes LGTM!

.card {
@apply card-base text-center;
@apply relative w-full border border-transparent mobileLandscape:w-120 mobileLandscape:bg-white rounded-xl px-8 py-9 mobileLandscape:shadow-card-grey-drop mb-6;
Copy link
Contributor

Choose a reason for hiding this comment

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

Yaaaaas

@@ -25,9 +25,10 @@ export const MOCK_AUTHPW =
'b5a61d1f7a6b1b762539bd85f783b65f635def1025c3f66fc51a438a68a77d6d';
export const MOCK_UNBLOCK_CODE = 'A1B2C3D4';
export const MOCK_RECOVERY_CODE = 'a1b2c3d4e5';
export const PLACEHOLDER_IMAGE_URL = 'https://loremflickr.com/512/512';
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice cleanup here too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

placekitten no longer works 😢

@vpomerleau vpomerleau merged commit 3ad136a into main Nov 15, 2024
26 checks passed
@vpomerleau vpomerleau deleted the FXA-10613 branch November 15, 2024 22:33
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