feat: introducing a custom element for rendering multiple product cards#1019
Draft
manikandan-ravikumar wants to merge 2 commits intomainfrom
Draft
feat: introducing a custom element for rendering multiple product cards#1019manikandan-ravikumar wants to merge 2 commits intomainfrom
manikandan-ravikumar wants to merge 2 commits intomainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request introduces a new DynamicCards component intended to render multiple product cards dynamically. However, the implementation has several critical issues that prevent it from functioning correctly:
Changes:
- Added DynamicCards component with support for loading products from Nosto recommendations and rendering them via Shopify sections
- Added utility files (productResultsParser.ts, markup.ts, styles.css) that are not integrated with the component
- Added Storybook stories that incorrectly reference the DynamicCard component instead of DynamicCards
- Exported DynamicCards and setDynamicCardsDefaults in main.ts
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 23 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main.ts | Exports the new DynamicCards component and its defaults setter function |
| src/components/DynamicCards/DynamicCards.ts | Main component implementation with issues in type definitions, documentation, naming, and base class selection |
| src/components/DynamicCards/DynamicCards.stories.tsx | Storybook stories that incorrectly render DynamicCard instead of DynamicCards |
| src/components/DynamicCards/styles.css | CSS styles for mock cards that are never imported or used |
| src/components/DynamicCards/markup.ts | Mock markup generator that is never imported or used |
| src/components/DynamicCards/productResultsParser.ts | DOM parsing utility that is never imported or used |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
This pull request introduces the new
DynamicCardsweb component, which enables the dynamic rendering of multiple Shopify products in a grid layout, along with related utilities, mock support, and Storybook stories. The main changes include the implementation of theDynamicCardscomponent, supporting utilities for markup and product parsing, styling, and documentation/demo setup.Key changes:
New Component: DynamicCards
DynamicCardsweb component (nosto-dynamic-cards) for rendering batches of products dynamically, with support for lazy loading, product recommendations, and batch fetching from Shopify sections. Includes a method to set global defaults for the component.Utilities & Helpers
Styling
Storybook & Documentation
DynamicCards, including controls for Shopify shop, template/section, product count, columns, and mock mode. This allows for interactive demos and visual testing of the component in various configurations.Related Jira ticket
Screenshots