-
Notifications
You must be signed in to change notification settings - Fork 18
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
[Feature] new layout/design #72
base: develop
Are you sure you want to change the base?
Conversation
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
@@ -92,13 +92,13 @@ helpers.dynamicPartial = function(name, partialData, options) { | |||
}; | |||
|
|||
// 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:
module
andstyleguide
page for easy switchingmodule
layout 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 onmodule
layout places the variants in a new right sidebarmodule
andstyleguide
received some new CSS with the target of better understanding which are the demo boundariesHow do you see this proposal?