-
Notifications
You must be signed in to change notification settings - Fork 18
[Feature] new layout/design #72
base: develop
Are you sure you want to change the base?
Conversation
backflip
left a comment
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 love it! Added some comments about details above.
@saschaeggi, could you have a look from a design perspective?
| @@ -0,0 +1,69 @@ | |||
| <style> | |||
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.
Can we move this to the preview's main.scss?
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 left it here on purpose for 2 reasons:
- I don't want to pollute the styles from Estatico and/or the project, specially considering that adding those styles in there will affect for sure modules and/or pages using in this case quotes.
- I think it is great to have it as an example here to see that Estático is also capable to show beautiful documentation
What do you think?
source/preview/layouts/layout.hbs
Outdated
| {{> preview/partials/tree }} | ||
|
|
||
| {{#if env.dev}} | ||
| <script async defer> |
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.
Can we move this block to a new script in preview/assets/js/?
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.
Sure
| @@ -0,0 +1,41 @@ | |||
| <footer class="sg__footer" role="contentinfo"> | |||
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.
Unsure about this, I'd rather remove it since I rather see this repo as a boilerplate.
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.
To me this is also part of it: It gives a way more professional view to the product, and it serves to me as default only.
Proposal: I'll add some HBS vars so that it gets automatically populated wit project info. Fine for you?
helpers/handlebars.js
Outdated
|
|
||
| // Module preview | ||
| helpers.hasVariants = function(variants, options) { | ||
| /*helpers.hasVariants = function(variants, options) { |
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.
Let's remove it if we don't use it anymore.
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.
Sure
A bit of reorganisation:
moduleandstyleguidepage for easy switchingmodulelayout has been cleaned because now every module has at least 1 variant. The targe is to go BEM, so the targe is to be able to define variants, themes, modificators and so onmodulelayout places the variants in a new right sidebarmoduleandstyleguidereceived some new CSS with the target of better understanding which are the demo boundariesHow do you see this proposal?