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 cart improvements empty state #319

Merged
merged 10 commits into from
Aug 9, 2021
Merged

Fix cart improvements empty state #319

merged 10 commits into from
Aug 9, 2021

Conversation

tauthomas01
Copy link
Contributor

@tauthomas01 tauthomas01 commented Aug 4, 2021

Why are these changes introduced?

Fixes #314

The goal of this PR is to improve the empty state of the cart.

image

What approach did you take?

  • Adjust CSS spacing
  • Add Liquid text and login link

Demo links

Checklist

@martinamarien martinamarien self-requested a review August 4, 2021 19:08
martinamarien
martinamarien previously approved these changes Aug 4, 2021
Copy link
Contributor

@martinamarien martinamarien left a comment

Choose a reason for hiding this comment

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

Looks good on my end Thomas. I'd recommend we request translations now if content is confirmed so that they come in soon 🤞🏻

templates/cart.json Outdated Show resolved Hide resolved
Comment on lines 22 to 27
{%- if shop.customer_accounts_enabled -%}
<h3 class="cart__login-title">{{ 'sections.cart.login.title' | t }}</h3>
<p class="cart__login-paragraph">
{{ 'sections.cart.login.paragraph_html' | t: link: routes.account_login_url }}
</p>
{%- endif -%}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this only meant to be visible on the empty cart state?
If it allows you to checkout faster, shouldn't we surface this to customer with items in their cart as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, this is empty state only. It's an interesting point, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc. @melissaperreault if you have time to see Martina's question.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a good question, I'll investigate with the team and come back! 👍

Copy link
Contributor

@tyleralsbury tyleralsbury left a comment

Choose a reason for hiding this comment

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

Looks good overall. One question, can we default the featured collection to the all_products collection? I think that should be possible. It would allow for merchants to not need to make any changes for it to already look good in the cart.

locales/en.default.json Show resolved Hide resolved
@martinamarien
Copy link
Contributor

I added a comment regarding that I am unsure if it's possible to display all products. Usually, we need to use an existing collection.

I wonder if we should do this if this is the only place we are doing something like this on the theme.

@tyleralsbury
Copy link
Contributor

I added a comment regarding that I am unsure if it's possible to display all products. Usually, we need to use an existing collection.

I wonder if we should do this if this is the only place we are doing something like this on the theme.

We can see with @Guillaumegranger1 but the rationale was basically that this is a part of the theme the merchant might not actually check when setting up their store, compared to something like the homepage, so we should have a default that makes sense.

If "all products" isn't possible, maybe we can default it to their first collection, whatever that is? Not sure if that works or not. 🤔

@tauthomas01
Copy link
Contributor Author

tauthomas01 commented Aug 5, 2021

If "all products" isn't possible, maybe we can default it to their first collection, whatever that is? Not sure if that works or not.

Usually, we use the frontpage (homepage) collection, but os2-demo removed it so I am not sure if it's reliable.

I tried all_products but it doesn't seem to work.

@tauthomas01
Copy link
Contributor Author

Update: I found out all is a value to display all collection by default, thanks to Sofia and Martina!

image

@tauthomas01 tauthomas01 mentioned this pull request Aug 5, 2021
3 tasks
@tauthomas01 tauthomas01 force-pushed the fix-cart-improvements branch from fbfa513 to 46b1d2e Compare August 5, 2021 15:22
@tauthomas01
Copy link
Contributor Author

Thank you for the review @melissaperreault ! Just made the changes

martinamarien
martinamarien previously approved these changes Aug 8, 2021
Copy link
Contributor

@martinamarien martinamarien left a comment

Choose a reason for hiding this comment

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

Looks good Thomas. All translations are back too 🙌🏻

tyleralsbury
tyleralsbury previously approved these changes Aug 9, 2021
Copy link
Contributor

@tyleralsbury tyleralsbury left a comment

Choose a reason for hiding this comment

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

Looks awesome.

]
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Feel free to keep this so that the approvals don't get removed. Just noting it.

@melissaperreault melissaperreault self-requested a review August 9, 2021 17:04
Copy link
Contributor

@melissaperreault melissaperreault left a comment

Choose a reason for hiding this comment

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

Once the little small spacing change are done, we are good to go! 🎉

@tauthomas01 tauthomas01 merged commit eaa5621 into main Aug 9, 2021
@tauthomas01 tauthomas01 deleted the fix-cart-improvements branch August 9, 2021 17:46
maplerock added a commit to fellowsmedia/nsra_shopify_2021 that referenced this pull request Aug 12, 2021
* main: (23 commits)
  Cart content update (Shopify#370)
  Footer spacing and alignment adjustments (Shopify#369)
  Product Template UI polish updates (Shopify#219)
  footer ui updates (Shopify#318)
  Fix cart improvements empty state (Shopify#319)
  [Announcement] Adjust block id for displaying dynamic names (Shopify#327)
  Update translations: buyer (Shopify#329)
  Revert editor setting changes (Shopify#328)
  Update translations (Shopify#294)
  Fix collection filtering UX (Shopify#268)
  Added page width setting and fixed image quality (Shopify#292)
  Add top border on cart notification when "show separator line" setting is deactivated (Shopify#306)
  movebadge code into base.css (Shopify#313)
  Collage UI bug fixes (Shopify#308)
  Customer account UI polish (Shopify#177)
  Added custom liquid block to product page (Shopify#269)
  Fix disclosure icon (Shopify#310)
  Fix facet price filter label spacing (Shopify#300)
  Fix duplicate search icon when header logo is set to "Top center" (Shopify#252)
  Add card outline setting (Shopify#239)
  ...
phapsidesGT pushed a commit to Gravytrain-UK/gt-shopify-dawn-theme that referenced this pull request Sep 3, 2024
* Fix cart improvements empty state

* rename featured_products to feature-products

* Add 'all' to show all products when displaying featured collection in default state

* Update 3 translation files

* add white space at end of line

* Adjust heading hierarchy, and spacing

* Update 21 translation files

* Update 6 translation files

* Spacing adjustment

* adjust padding

Co-authored-by: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com>
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.

Cart empty state enhancement
6 participants