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

TH-230: Add Products-by-Collection section #147

Merged
merged 30 commits into from
Jan 3, 2025
Merged

TH-230: Add Products-by-Collection section #147

merged 30 commits into from
Jan 3, 2025

Conversation

eihabkhan
Copy link
Member

@eihabkhan eihabkhan commented Dec 31, 2024

JIRA Ticket

Ticket: TH-230.

Prerequisites

  • Check this branch locally and run pnpm run release
  • 3 new files will be generated in this format theme name + date + .zip
  • Upload generated file locally or in seller-area test env

QA Steps

  • Go to seller-area
  • make sure you have a collection with products in it
  • Once that's done go to theme editor (Edit Origins)
  • You should see a "products by collection" section
  • Click on it and start customizing it.
  • The customization options should be the following:
    • Collection
    • Title (Section)
    • Description (Section)
    • Header position: Left, Right, Center
    • View Style: Grid, or Slides
    • Text Color
    • Background color
    • Maximum products to show
    • Number of columns on desktop
    • Show link to collection
    • Enable quick add (This setting won’t work since we don’t have neither the cart or the modal logic yet)
    • Section Spacing (Top, bottom)
  • Play with those customization settings, try to break it.

Note

You might find some responsitivity issues when the section is in "Slides" mode. That's an issue with our core slider and will be fixed in the PR of this ticket.
Another issue arose where the contents of the slides are not responsive to user clicks (Ticket).

@eihabkhan eihabkhan self-assigned this Dec 31, 2024
@eihabkhan eihabkhan added the in progress Currently being developed. label Dec 31, 2024
@eihabkhan eihabkhan added Ready for review Requires a review from another developer. and removed in progress Currently being developed. labels Jan 2, 2025
@eihabkhan eihabkhan marked this pull request as ready for review January 2, 2025 09:44
Copy link
Contributor

@ibrilBadreddine ibrilBadreddine left a comment

Choose a reason for hiding this comment

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

Nice work Mr Eihab, here are some notes/suggestions to refine this section:

  • maybe a scale effect on product image hover could work well?
  • looks like secondary fits better than icon for the product button variant.
  • product titles need a min-width.
  • global product settings don’t fully work (currently, only card style applies)
  • compact product style is missing a background
  • The show more button isn’t visible in slides view style.
Screenshot 2025-01-02 at 14 41 44 Screenshot 2025-01-02 at 14 41 49 Screenshot 2025-01-02 at 14 45 58 Screenshot 2025-01-02 at 14 49 08

themes/origins/styles/component-slider.scss Outdated Show resolved Hide resolved
themes/origins/styles/products-by-collection.scss Outdated Show resolved Hide resolved
Comment on lines 16 to 25
.yc-btn {
width: fit-content;
color: currentColor;
border: var(--product-card-border);
background-color: var(--product-card-background);

&:hover {
background-color: rgba(var(--product-card-foreground), 0.06);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Question is who's gonna merge first 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

rock paper scissors :sadpablo

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I won :kekw

@eihabkhan eihabkhan merged commit 9435c30 into rabi3a Jan 3, 2025
@eihabkhan eihabkhan deleted the TH-230 branch January 3, 2025 11:06
@ibrilBadreddine ibrilBadreddine added Reviewed PR has been reviewed and approved. and removed Ready for review Requires a review from another developer. labels Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Reviewed PR has been reviewed and approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants