-
Notifications
You must be signed in to change notification settings - Fork 142
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
Back-port Shopper Experience Base Components into Retail Template #992
Conversation
Page.displayName = 'Page' | ||
|
||
Page.propTypes = { | ||
page: PropTypes.object.isRequired, |
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.
Will the API response always include a page
key?
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.
No it wont. It's up to the developer to take the page returned from the API and pass it to the page component under the prop "page". I did this because there will potentially be other properties, for example, if we decide to add some kind of edit mode you might have a property called "editing". Also we are passing in className, and other props that get forwarded to the main container.
Region.displayName = 'Region' | ||
|
||
Region.propTypes = { | ||
region: PropTypes.object.isRequired, |
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.
Does the API response use regions
plural, not region
?
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.
The pages "regions" are mapped over in the page component itself. I chose originally to make the components singular, I believe this is more versatile as developers could potentially use the Region component on it's own, instead of having to render all the regions.
switch (prop) { | ||
default: | ||
componentClass = (props) => ( | ||
<div style={{marginBottom: '10px'}}> |
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.
I'm guessing all this code will be removed before merging.
Any idea why we see a style warning in the browser console?
Did not expect server HTML to contain the text node ".css-niwogb{color:var(--chakra-colors-black);}" in <select>
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.
LGTM
* Back-port Shopper Experience Base Components into Retail Template (#992) * Backport experience base components into template * Fix linting * Fix test imports * Make types more robust * Remove temp example page * remove updatePw from not implemented list (#996) --------- Co-authored-by: Ben Chypak <[email protected]> Co-authored-by: Alex Vuong <[email protected]>
* develop: Support Node 16 (#965) [W-12450361] Introduce short-circuit method for bypassing auth in Commerce Provider by passing in a fetchedToken (#1010) remove jest-silent-reporter (#1009) Clean up Page Designer Code (#1004) [Shopper Experience] `ImageWithText` component (#991) Feature: Page Designer Carousel Component (#977) [Feature] Page Designer Layout Components WIP (#993) remove updatePw from not implemented list (#996) Back-port Shopper Experience Base Components into Retail Template (#992) # Conflicts: # packages/commerce-sdk-react/package-lock.json # packages/commerce-sdk-react/package.json # packages/pwa-kit-dev/package-lock.json # packages/pwa-kit-dev/src/configs/webpack/config.js # packages/template-retail-react-app/package-lock.json
This PR does 2 things. Ports the shopper experience component from
commerce-sdk-react
into the retail template. 2. Enabled the Shopper Experience api in commerce-api client.How to test:
Run the dev server and goto the /page-viewer path. It should display a hierarchical view of the
homepage-example
page.Before merging, remove the test page.