spike: Example of custom button through intercept #2848
Conversation
If your PR is missing information, check against the original template here. At a minimum you must have the section headers from the template and provide some information in each section. LogERROR ON TASK: scaffoldingSucceeds |
Signed-off-by: sirugh <rugh@adobe.com>
sirugh
force-pushed
the
rugh/spike-theme-via-local-intercept
branch
from
8c736a6
to
5f75e4b
Compare
| ); | ||
| button.replaceJSX( | ||
| 'button', | ||
| `<${CustomButton} className={rootClassName} type={type} disabled={disabled} {...restProps}>{children}</${CustomButton}>` |
There was a problem hiding this comment.
Preferably we would just overwrite the export of the Button component with the CustomButton rather than replacing the JSX which requires knowledge of the props/children structure.
@jimbo suggested:
// before
export { default } from "./button"
// after
export { default } from "@magento/venia-concept/src/components/CustomButton"
93f1c46
to
456a831
Compare
| const storeCode = storage.getItem('store_view_code'); | ||
| const [css, setCss] = useState(defaultClasses); | ||
| useEffect(() => { | ||
| if (storeCode === 'fr') { | ||
| loadCss(); | ||
| } |
There was a problem hiding this comment.
I'm not sure how I feel about components having an understanding of the store view code. I wonder if there'd be some mechanism of doing this at a framework level so we don't end up with store view codes in components. Just food for thought :)
There was a problem hiding this comment.
How would you know which css to load for which store code? Something will have to know, right? Also, this "CustomButton" is a custom component from the client, not built into PWA Studio. You can only do such extension through scaffolding so it's in venia-concept as an example.
There was a problem hiding this comment.
It really depends what we refer to when we say theming, this could mean changing one or two components, changing some CSS, or having a completely different tree of components with inheritance.
Would be good to validate legitimate business cases of theme customization and see where they align.
There was a problem hiding this comment.
Yea, the point of this spike PR was just to explore dynamic CSS loading. My take is that customization and theming are going to continue to fall into the realm of our developer users. The extension framework allows for so much customization and granularity that it seems counter productive to implement some rigid structure around what we consider theming.
Anyways, I agree that we need to understand the business cases before implementing anything concrete. I've written up my thoughts in the JIRA ticket.
m2-community-project
bot
moved this from Ready for Review
to Review in Progress
in Pull Request Progress
m2-community-project
bot
added
Progress: review
and removed
Progress: ready for review
labels
Signed-off-by: sirugh <rugh@adobe.com>
Signed-off-by: sirugh <rugh@adobe.com>
| .root { | ||
| color: red; | ||
| border-color: red; | ||
| } |
There was a problem hiding this comment.
My intent was just for this to be the css that was needed. Should be possible to provide just those overrides without having to know all the rest of the classes.
| <button | ||
| // The only other custom thing here...But there might be a better | ||
| // way to ensure that incoming classes are used. | ||
| className={`${className} ${rootClassName}`} |
There was a problem hiding this comment.
I think it would be preferable to just modify the Button component itself with this and the changes from line 25. No copy of Button would be necessary, just need to import the hook and pass it the component name + default classes.
|
Closing this as I have explored the idea enough. |
Just a spike to try out one approach to themes through the scaffolded intercept file.
The intercept file replaces all instances of
Buttonwith theCustomButtoncomponent which is identical except for a dynamic load of the css used. Whenstore_codeisfr, it loadsfrenchButton.cssand uses those classes instead of the default.Try it out by switching the store to "French Store View".