-
Notifications
You must be signed in to change notification settings - Fork 7
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 theme, router and current product, React Hooks and HOC to easily access the appropriate data #616
Add theme, router and current product, React Hooks and HOC to easily access the appropriate data #616
Conversation
…ct related information.
Pull Request Test Coverage Report for Build 2455
💛 - Coveralls |
…simple-custom-page-route-helper # Conflicts: # extensions/@shopgate-product-reviews/frontend/package.json # themes/theme-gmd/package.json # themes/theme-ios11/package.json
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 think, this PR doesn't cover the requirement from the story yet. Using both contexts is now really simplified, but one still needs to import and use them explicitly.
What about extending the default Route component with additional props to activate the context injection without importing extra stuff? We could also provide a new component.
I got some feedback from PS, that this could definitely improve the workshops, since they don't need to explain "new" terms like "HOD" or "Hooks".
libraries/engage/core/index.js
Outdated
@@ -140,7 +145,7 @@ export { default as YouTube } from '@shopgate/pwa-common/collections/media-provi | |||
|
|||
// --------------- CONTEXTS --------------- // | |||
|
|||
export * from '@shopgate/pwa-common/context'; | |||
// export { RouteContext, ThemeContext } from '@shopgate/pwa-common/context'; |
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.
Why did you replace this line by commented out code? Is it just because externals are now supposed to use the hooks / HOC?
In the recently updated DevDocs we have some articles where import { Theme } from '@shopgate/engage/core';
is used within code snippets. This will not work anymore when the export is gone. So you also need to update the developer center articles with the new patterns from this PR.
I actually talked that through with @sznowicki from PS and we confirmed that this will be the solution to go for. |
Description
In order to easily create custom Routes and access the product data very easily, we added three new React Hooks:
useTheme
- Provides all theme components that are defined in the Theme APIuseRoute
- Provides all route parameters that are useful for render controllinguseCurrentProduct
- Provides the current product context information on the Product Detail PageAdditionally all this information can be accessed with HOC:
withTheme
withRoute
withCurrentProduct
Type of change
Please add an "x" into the option that is relevant: