Skip to content
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

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

[Feature] new layout/design #72

wants to merge 7 commits into from

Conversation

orioltf
Copy link
Member

@orioltf orioltf commented Jun 9, 2017

A bit of reorganisation:

  • The index page is now a partial that is included as a sidebar in every module and styleguide page for easy switching
  • The module 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 on
  • The module layout places the variants in a new right sidebar
  • The module and styleguide received some new CSS with the target of better understanding which are the demo boundaries
  • The index page can now be used for general documentation/marketing purposes. By default it kinda says what is Estático, feel free to propose what would you like we to have in there by default. I can see many potential however, for project oriented information.

How do you see this proposal?

@orioltf orioltf requested a review from backflip June 9, 2017 15:32
Copy link
Collaborator

@backflip backflip left a 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>
Copy link
Collaborator

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?

Copy link
Member Author

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:

  1. 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.
  2. 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?

{{> preview/partials/tree }}

{{#if env.dev}}
<script async defer>
Copy link
Collaborator

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/?

Copy link
Member Author

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">
Copy link
Collaborator

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.

Copy link
Member Author

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?

@@ -92,13 +92,13 @@ helpers.dynamicPartial = function(name, partialData, options) {
};

// Module preview
helpers.hasVariants = function(variants, options) {
/*helpers.hasVariants = function(variants, options) {
Copy link
Collaborator

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants