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

login is aligned with nav links when language & country unselected #2743

Merged
merged 5 commits into from
Jun 29, 2023

Conversation

lougoncharenko
Copy link
Contributor

PR Summary:

When mobile drawer is login only, the login link is aligned with the nav links.

Why are these changes introduced?

Fixes #2671 .

What approach did you take?

I used a css class to apply the margin-left when the utility links were missing the localization form but had the account.

Other considerations

N/A

Visual impact on existing themes

If merchants choose to unselect the country and language selector, the account login button will be aligned on the mobile version.

Testing steps/scenarios

  • View Mobile drawer with country and language selectors checked
  • View Mobile drawer with country selector checked
  • View Mobile drawer with language selector checked
  • View Mobile Drawer with both country and language selectors unchecked (account only)
  • View Mobile Drawer with the social icons and account only

Demo links

Checklist

@lougoncharenko lougoncharenko linked an issue Jun 27, 2023 that may be closed by this pull request
7 tasks
Copy link
Contributor

@kmeleta kmeleta left a comment

Choose a reason for hiding this comment

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

LGTM! Just a couple small code suggestions.

@lougoncharenko lougoncharenko requested a review from kmeleta June 27, 2023 16:56
@kimberlyoleiro
Copy link

Hey! *Nit-pick

Curious why the padding is 30px when lang/currency is present, but 20px + 10px margin without it?
Group 5

@lougoncharenko lougoncharenko requested a review from kmeleta June 28, 2023 12:35
@lougoncharenko lougoncharenko self-assigned this Jun 28, 2023
Copy link
Contributor

@ludoboludo ludoboludo left a comment

Choose a reason for hiding this comment

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

Looking good. I'm questioning the values we're using though. As we have both 2rem 3rem and 3rem. I think we could maybe be consistent.

And then I wanted to mention that there is one more test scenario you can check (I did and it looked good 👍) is when you disable account on the store.
It's not controlled at the theme level but in the admin settings. Here is a little video on how you'd go about disabling/enabling accounts on the storefront.

Copy link
Contributor

@ludoboludo ludoboludo left a comment

Choose a reason for hiding this comment

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

🎉

@lougoncharenko lougoncharenko merged commit 1598559 into main Jun 29, 2023
@lougoncharenko lougoncharenko deleted the fix-login-alignment-bug-on-mobile-drawer branch June 29, 2023 15:19
phapsidesGT pushed a commit to Gravytrain-UK/gt-shopify-dawn-theme that referenced this pull request Sep 3, 2024
…hopify#2743)

* login is now aligned with nav links when language and country selectors are unselected

* fixed alignment on account & social icons

* applied feedback

* changed utlitility links to be 3rem vs 2rem +1rem from margin-left padding

* implemented hanges from code review
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.

Mobile drawer UI fix
4 participants