-
Notifications
You must be signed in to change notification settings - Fork 249
Conversation
So far so good, I didn't see any different from this and the old boilerplate. Am I right ? Just out of curiosity, do you have plan to let user swap out compatible theme with 1 line of config ? |
You probably didn't look enough. There are a lot of noticeable differences if you look at the tree.
On the long run yes, but for now we are far from that (maybe not that far but still). |
44000f6
to
1d4f64f
Compare
PR updated. Should be good now. |
|
||
render() { | ||
return ( | ||
<div className={ styles.container }> |
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 is unnecessary, can we just move the styling part to AppContainer ?
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 to keep a simple AppContainer without any CSS.
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.
OK. 👍
Except for my comments. LGTM |
We encourage you to update to a similar structure if you were using the previous one. Main changes: - ``web_modules/layouts`` => ``src/layouts`` - ``web_modules/{Components}`` => ``src/components/*`` - ``web_modules/app/*`` => ``src/*`` - ``web_modules/LayoutContainer`` => ``src/AppContainer.js`` Closes #529 Closes #444
To prevent issue in the future, is now accessible by doing ``import { PageContainer } from "phenomic"``. If you want to import it with a different name, you can do it this way: ```js import { PageContainer as PhenomicPageContainer } from "phenomic" ```
…iv>`` if a single string is passed as a child.
bd1ded5
to
19b21e8
Compare
Closes #529
Closes #444
Will closes #433