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

#346 Start building Design System - Style Guide #387

Merged

Conversation

sandrina-p
Copy link
Contributor

@sandrina-p sandrina-p commented Apr 27, 2018

Following #346,

I've created a new page /design-system with the first part: Style Guide.

The titles are the same from Bulma but only the first 5, let's try not use 6 and 7, so we don't have too many font-sizes.

The text is pretty simple. Normal, grayer and smaller. We could create a is-secondary and is-small modifiers, since Bulma doesn't support an more intuitive way of doing this transformation.

I've noticed Bulma is lacking on responsive $gap variables (using rem instead of px), as well variants, so I've improved it with our own variables $spacer with 3 variants.

The colors have the 3 brand default colors (blue, green and orange) as primary, secondary, tertiary. I've used color functions (darken, saturate, etc...) to find the best colors to suit success/warning/danger. Still, we need to work better on those colors, because they fit differently on text and background.

image

Regarding theming, CSS Variables could be the way but we need to fork Bulma and adapt its structure to support CSS Variables. I've found an article explaining very well tips/tricks about working with SCSS and CSS Variables, we could give it a shot. Otherwise the only solution I see is somehow generate a different css bundle for each theme file found at sass/theme (right now only default available). If we will customize just the colors, I believe the variables on that file are a good start.

Once again, I've cleaned up some more CSS code found on components that I've crossed with.

Important: that in order to use SCSS variables on vue components we need to import @import "../sass/theme/index"; so all Bulma and GI variables/utilities are available to use, even if the active theme changes, it will work correctly.

@sandrina-p sandrina-p force-pushed the feat/346-define-gi-style-guide branch from bacb3f1 to b9ab9f1 Compare April 27, 2018 22:21
@sandrina-p sandrina-p requested a review from hubudibu April 27, 2018 22:21
@taoeffect
Copy link
Member

This seems like a useful thing to have around, especially if it gets periodically updated with various widgets that we end up using (e.g. buttons etc). 👍

(Just remember to not spend too much time on this, as that can lead down the road of meta-work...)

Regarding theming, CSS Variables could be the way but we need to fork Bulma and adapt its structure to support CSS Variables. I've found an article explaining very well tips/tricks about working with SCSS and CSS Variables, we could give it a shot. Otherwise the only solution I see is somehow generate a different css bundle for each theme file found at sass/theme (right now only default available). If we will customize just the colors, I believe the variables on that file are a good start.

Forking Bulma is definitely an option (and a welcome one). It will mean the end of having to keep up with their updates, and it will mean we can greatly reduce the size of the CSS we're having to manage in the app. Getting rid of dependencies is also something that needs to be an ongoing process to close issue #372, so I'm all for forking Bulma, especially if you think that's the cleanest / most correct / simplest way forward.

Important: that in order to use SCSS variables on vue components we need to import @import "../sass/theme/index"; so all Bulma and GI variables/utilities are available to use, even if the active theme changes, it will work correctly.

This is not clear to me... are you saying that line needs to appear on all vue components? Surely that's not necessary since there's a top-level CSS file?


$body-background-color: #fff;
$radius: 3px;
$gap: 16px;
Copy link
Member

Choose a reason for hiding this comment

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

I would expect files within the theme folder to just be theme files.

And the above does not look like structure css, but as we discussed on the call "colors and typography", so therefore I'm assuming this is the "kindergarten professional theme". If so, it needs to be named as such, not default.scss. The "default" theme must be set by a variable/setting in JavaScript somewhere. See also recently created issue #388.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Your are right, I've moved $radius and $gap to bulma_overrides/utilities, so this file, no named theme/kindergarten-professional.scss keeps only the variables related to the theme.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW, did you see the notice at the top of that post?

UPDATE April 2016: I wrote up a new approach for this with fallbacks CSS4 Variables with Fallbacks Using Sass.

Thoughts on that?

Yes I did, and it's similar to those changes that we need to integrate on Bulma. Meanwhile, I've created an issue on Bulma asking if they have a date in mind to add CSS Variables support, since it's on their roadmap. Maybe we can wait for it before forking it. What do you think?

@taoeffect
Copy link
Member

I've found an article explaining very well tips/tricks about working with SCSS and CSS Variables, we could give it a shot.

BTW, did you see the notice at the top of that post?

UPDATE April 2016: I wrote up a new approach for this with fallbacks CSS4 Variables with Fallbacks Using Sass.

Thoughts on that?

@sandrina-p sandrina-p changed the title Start building Design System - Style Guide #346 Start building Design System - Style Guide Apr 28, 2018
@sandrina-p sandrina-p force-pushed the feat/346-define-gi-style-guide branch from 4ba305c to 21a1c77 Compare April 28, 2018 18:14
@taoeffect
Copy link
Member

Yes I did, and it's similar to those changes that we need to integrate on Bulma. Meanwhile, I've created an issue on Bulma asking if they have a date in mind to add CSS Variables support, since it's on their roadmap. Maybe we can wait for it before forking it. What do you think?

See my reply

@taoeffect
Copy link
Member

@sandrina-p now that #355 recently merged, this PR has conflicts

@sandrina-p sandrina-p force-pushed the feat/346-define-gi-style-guide branch from 21a1c77 to 16170a9 Compare April 29, 2018 01:13
@sandrina-p
Copy link
Contributor Author

@taoeffect conflits fixed

Copy link
Contributor

@hubudibu hubudibu left a comment

Choose a reason for hiding this comment

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

👍


<div class="column is-4">
<h2 class="title is-4 gi-title">Text</h2>
<p>In an ideal world this website wouldn’t exist, a client would acknowledge the importance of having web copy before the design starts. Needless to say it’s very important, content is king and people are beginning to understand that. </p>
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this sad copy is relevant here, can we have some happy lorem ipsum on a design system page instead? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done :D

@sandrina-p sandrina-p force-pushed the feat/346-define-gi-style-guide branch from aebe410 to f74203e Compare May 1, 2018 20:49
@sandrina-p sandrina-p force-pushed the feat/346-define-gi-style-guide branch from f74203e to ed4cad7 Compare May 1, 2018 20:50
@taoeffect taoeffect merged commit 3bf4dea into okTurtles:master May 2, 2018
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.

3 participants