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

Add card outline setting #239

Merged
merged 9 commits into from
Aug 3, 2021
Merged

Add card outline setting #239

merged 9 commits into from
Aug 3, 2021

Conversation

KaichenWang
Copy link
Contributor

Why are these changes introduced?

Fixes #189

What approach did you take?

  • Add Show image outline setting to sections:

    • main-collection-product-grid
    • featured-collection
    • main-search
    • product-recommendations
  • Add logic in product-card snippet to conditionally show image outline

  • Update CSS

  • Updates locales

Other considerations

  • Note that product cards without images do not have an outline

Demo links

Checklist

@tauthomas01 tauthomas01 self-assigned this Jul 22, 2021
Copy link
Contributor

@tauthomas01 tauthomas01 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. I found one small bug, but the rest looks good.

sections/main-search.liquid Show resolved Hide resolved
…outline-setting

# Conflicts:
#	sections/main-search.liquid
tyleralsbury
tyleralsbury previously approved these changes Jul 27, 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.

Tested and code looks good. For anyone testing on the demo store, it's a very subtle effect because of the background colour on the demo store images so it might be hard to see without inspect element.

tauthomas01
tauthomas01 previously approved these changes Aug 2, 2021
…outline-setting

# Conflicts:
#	sections/main-collection-product-grid.liquid
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.

Quickly re-tested and it looks good.

@KaichenWang KaichenWang merged commit 9ee99da into main Aug 3, 2021
@KaichenWang KaichenWang deleted the add-card-outline-setting branch August 3, 2021 15:34
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
* Add setting to product-grid to toggle product card outline

* Update all instances of product-card snippet

* Simplify liquid logic for showing outline

* Fix typo

* Update 11 translation files

* Update 7 translation files

* Update 2 translation files

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.

Add a setting to remove the line in the Collection list
3 participants