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

fix(fxa-settings): align password CTA #12492

Closed
wants to merge 1 commit into from
Closed

fix(fxa-settings): align password CTA #12492

wants to merge 1 commit into from

Conversation

millmason
Copy link
Contributor

@millmason millmason commented Apr 14, 2022

Because:

  • Since all other UnitRow components in the Security section have
    refresh buttons, the Password row CTA button seems out of alignment.

This commit:

  • Adds a descriptive prop so that the user can align the CTA within a
    UnitRow with buttons that have a refresh button.

Closes #11972

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)

Before:
Screen Shot 2022-04-14 at 12 04 18 PM

After:
Screen Shot 2022-04-14 at 12 06 27 PM

RTL (before changes):
Screen Shot 2022-04-14 at 12 48 47 PM

Other information (Optional)

Styles deliberately do not have the rtl prefix, as the position of the buttons does not change when the text direction of the page changes, as shown in the above screenshot. Has been tested in Arabic and Hebrew.

Looking at the new button alignment, I think it might be worth leaving the designs as they originally were -- to my eye the buttons have a slight optical illusion of being misaligned (even when they do line up) due to the buttons above the Security section all lining up flatly with the right-hand side of their container. I would really appreciate eyes + opinions on it!

Because:

* Since all other UnitRow components in the Security section have
  refresh buttons, the Password row CTA button seems out of alignment.

This commit:

* Adds a descriptive prop so that the user can align the CTA within a
UnitRow with buttons that have a refresh button.

Closes #FXA-4631
@millmason millmason marked this pull request as ready for review April 14, 2022 20:03
@millmason millmason requested a review from a team as a code owner April 14, 2022 20:03
@LZoog
Copy link
Contributor

LZoog commented Apr 14, 2022

@millsoper 1st PR woot! 🎉

I know I just mentioned in Slack we need to push our branches to mozilla/fxa directly, and I think we'll have to close this and re-open it for Storybook to deploy. We could also see if screenshots will be sufficient on determining if we want this or not, though. 🤷‍♀️ The code changes LGTM.

@millmason
Copy link
Contributor Author

Closing PR since I pushed it directly to mozilla/fxa per @LZoog 's comment above. New PR is at #12514

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.

[FxA-Settings] Change button from password section is not aligned with the other buttons
2 participants