-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat(snap-platforms) #1022
feat(snap-platforms) #1022
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the way.
Need to clean up the readme files and error logs to remove plugin references.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking good. I'd like to discuss some more about the "callback" and async usage of these addToCart
functions before we set the interface/usage in stone.
…ng components & option mappings
…ries for variantselection
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Types
- add
available
in ListOption type - refactor List, Grid, Swatches to new type - add
- need class modifiers in components based on ListOption value
--disabled
--unavailable
- styling should be the same for class modifiers above
- disabled options should not be clickable
export type ListOption = {
value: string | number;
label?: string;
disabled?: boolean;
available?: boolean;
default?: boolean;
icon?: string | Partial<IconProps>;
url?: UrlManager;
};
Grid Component
- List component has a 'title' why not this one?
- need story for overflowing grid and for options with backgrounds (image or whatever)
- prop for hiding "show less" (when you don't want to show less)
- :before pseudo element for "disabled" strike should not have fixed width
List Component
- remove
clickableDisabledOptions
- instead of padding lets use
gap
from flex disabled
still allows for selections- when using "horizontal" we should put the title
<h5>
next to<ul>
by adding styles to main wrapper:display: flex;align-items: center;
Swatches Component
- has a fixed width of 40px per slide, why?
- :before pseudo element for "disabled" strike should not have fixed width
- :before pseudo element doesn't work on white swatch
- disabled styling differs from grid / carousel - maybe unify?
VariantSelection Component
- storybook type selection doesn't work
- dropdown type needs
position: relative;
for disabled strike - for disabled class, maybe use simpler 'line-through' for styling?
…on and children components
…es where values are strings
… the swatches componentry
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could use some final review and testing.
…' config for async/await usage
…anges during final review
…sultStore and refactoring
Cartstore Price Fix
… when the config is undefined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
snap-platforms added for shopify, bigcommerce, & magento2