-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Update admin color palette and font #3192
Update admin color palette and font #3192
Conversation
Great work! Again I'd like to suggest that we drop the spree_frontend altogether and start working on starter kits in react/vue.js etc. |
I agree that this will more attractive to decision makers, and I am happy so much care is going into the backend. I hope we will still keep font-awesome or some other stock icon library easily available for developers. It is great to have ready-made icons available for extensions and custom backend views that Solidus does not provide out of the box. |
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.
Great work! 🚀
I added a couple of minor things to be fixed, after that seems ready for a merge 👍👍👍
backend/app/assets/stylesheets/spree/backend/globals/_fonts.scss
Outdated
Show resolved
Hide resolved
backend/app/assets/stylesheets/spree/backend/_bootstrap_custom.scss
Outdated
Show resolved
Hide resolved
backend/app/assets/stylesheets/spree/backend/components/_messages.scss
Outdated
Show resolved
Hide resolved
backend/app/assets/stylesheets/spree/backend/globals/_variables_override.scss
Outdated
Show resolved
Hide resolved
Hi @benjaminwil thank you for your feedback! |
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 looks great imo! 👍 Thanks @mfrecchiami!
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.
Looks awesome, great improvements! Obviously more improvements can be made as an iteration on this but this is a great start.
@jacobherrington and @ericsaupe thank you! 🙏 |
5592e2f
to
3cd2ef5
Compare
$color-4: #6788A2 !default; // Dark Blue | ||
$color-5: #C60F13 !default; // Red | ||
$color-6: #FF9300 !default; // Yellow | ||
$color-7: #F4F4F4 !default; // Light Gray |
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've just one concern about removing variables. If extensions or stores are using them they'll end up having an error while compiling stylesheets. What about moving them into a _deprecated-variables.scss
file instead?
I'm not sure if there's a way to actually deprecate them with scss. I've made some test with this code:
@if variable-exists(color-1) {
@warn '$color-1 will be removed, please use $color-primary;';
$color-1: $color-primary;
}
and it works, but it only prints a deprecation message if you re-define $color-1
, not if you just use it. It also has to be declared after the $color-1
definition, which is basically useless for both extensions and stores since they will eventually declare/override variables after solidus files are loaded.
@tvdeyen do you have any idea around this?
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.
Great point @kennyadsl
We already have a way to deprecate variables https://github.com/solidusio/solidus/blob/master/backend/app/assets/stylesheets/spree/backend/globals/_deprecation.scss
You use it like this
@include solidus-deprecated-variable("color-success", "brand-success"); |
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.
Hi guys! I've just pushed a commit to resolve this issue; as you can seen the old numbered color variables has been re-added in a their own scss partial, and I used the existing sass mixin, suggest by @tvdeyen (I just duplicate it and renamed the old one because it refers to Bootstrap variables, and its warning message, also).
Thank you @kennyadsl for your feedback!
The message that I wrote refers to a future 3.0 version of Solidus, so we can keep the deprecated variables on 2.8 and 2.9vs. What do you think about that?
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.
Awesome 🍰
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 awesome work. It really looks nice and great work implementing the change.
There are still some places where we could make use of Sass variables. Especially the button styles.
backend/app/assets/stylesheets/spree/backend/components/_navigation.scss
Outdated
Show resolved
Hide resolved
backend/app/assets/stylesheets/spree/backend/shared/_layout.scss
Outdated
Show resolved
Hide resolved
backend/app/assets/stylesheets/spree/backend/components/_breadcrumb.scss
Outdated
Show resolved
Hide resolved
backend/app/assets/stylesheets/spree/backend/components/_navigation.scss
Outdated
Show resolved
Hide resolved
backend/app/assets/stylesheets/spree/backend/_bootstrap_custom.scss
Outdated
Show resolved
Hide resolved
backend/app/assets/stylesheets/spree/backend/shared/_forms.scss
Outdated
Show resolved
Hide resolved
backend/app/assets/stylesheets/spree/backend/shared/_typography.scss
Outdated
Show resolved
Hide resolved
backend/app/assets/stylesheets/spree/backend/globals/_fonts.scss
Outdated
Show resolved
Hide resolved
There are still some places that use the old button classes and therefore get the secondary button style. Product -> Images for instance. BTW: I really like the secondary button style, as it looks like a button (a mayor downside of the flat UI trend). But it looks a bit misplaced next to the primary buttons that are still flat. Any plans to make them non-flat buttons as well? I would appreciate that. |
I agree with Benjamin here. Lots of stores and extensions use the provided icons. We can't just replace them without breaking them.
I understand the thought from a brand standpoint, but we are an open source framework that is used because of its extensibility and ease of customisation. That includes icons IMO. Custom drawn icons is a maintenance burden for us. I advice us to use open source icons whenever possible. FontAwesome 5 is a great resource for free icons that has lots of modern integration options. Please keep that in mind. Thanks for all the great work on the new admin. I really appreciate the work that has been done here. |
2a0b54f
to
e8832d0
Compare
e8832d0
to
9e9ba78
Compare
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.
Thanks for addressing 🍰
I have some questions about the font files. As we do not "made" them we should move them into the vendor/assets/fonts
folder and require them with an absolute path.
This is really looking great and we should be able to merge this soon :)
backend/app/assets/stylesheets/spree/backend/shared/_fonts.scss
Outdated
Show resolved
Hide resolved
559f373
to
dedb48b
Compare
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.
🚀 💯
18ed32c
to
0e82fe5
Compare
Update of color variables with the aim of making the product more recognizable. Application of the new colors to the navigation sidebar and tabs navigation.
Update logo with mark, space adjustment. Set the link click on the entire surface of the logo. Apply this changes on frontend, also.
Check and find right variables partial that is using to ovveride the Bootstrap default variables. This is necessary to update colors of buttons and minor components, later.
Apply the same new UI to some minor components like breadcrumbs and pagination. Set the same font weight of sidebar navigation items, to the bottom login navigation.
For now we are able to update the secondary action like Cancel.
While the new gray style did applied to the generic .button class, in common cases (and where the .btn-primary is applied on a secondary action) is better using the .btn-default class, suggested by Bootstrap.
In the previous commit it was introduced a new class to apply to the secondary actions that not has the generic .button class, but have the .btn-primary class (also if they are secondary actions, e.g.: Cancel).
Add a 4px blue line on the top of header, because in the second phase we want to have a minimal darken header without breadcrumbs inside it. Then, some top margin was added to main navigation.
Avoid legend titles and total amounts with green color: first reason is because of legibility, then is better use green color only for success messages or status.
The background and background-color conflicted when are used on .fa class
Add a border to popover arrow, change background color and add border-radius
Always folllowing the aim of create a product image for Solidus, the Open Sans font has been replaced by Inter, a open source font which stands out for its readability and likeness to the SF Apple font (not an open source font, actually) that is in use on Solidus website
Apply to input-border variable its own color (a darken gray vs previous border color); adjust the color of error messages, also to the validation state form. Adjust flash states messages colors. The next step for the flash messages is to insert a related icon (e.g.: check icon for success messages etc.).
After these changes is now the time for update the Backend Style Guide, with brand new colors.
While we are changing the colors palette, we've decided to give conventional names to the main colors variables that were: color-1, color-2, color-3 and so on. Now we can recognize the colors easily. The style guide has also been updated.
A text-transform has been added to style the Store Location Name, but changhe this name to uppercase, fails the Circle test.
All the HEX colors and font weight style has been replaced with their variable names. Furthermore an oversight about the new font has been fixed.
I had to add a rule to the production.rb file for allow the correct interpretation of the font formats.
Previously numbered color variables has been re-added in a their own scss partial, so we can show a deprecation warning message that suggests to change e.g.: `$color-1` to `$color-white`. The name of the existing mixin has been changed also, because its warning message was about Bootstrap variables.
The btn-primary style has been applied on the primary actions visible at the top of some secrtions.
The previous implementation for the rspec custom matcher `appear_before` was often flaky. The new one, relying on the standard Capybara matcher `#has_text?` is supposed to be more reliable, also when JS is involved.
In the relative previous commit, the old numbered color variables has been re-added for print a custom warning message; actually the message was printed every time and in any case, and it was not so helpful. Now, the warning msg is printed only if a variable is declared in `_variable_override.scss`.
c1ee6b6
to
c5fb960
Compare
Great job! |
Indeed, thank you for that! |
With the aim of defining a solid brand image for Solidus, we updated the color palette and the backend font with Inter, a 100% Open Source font family; then we'll replace the product images with the Solidus design, then all the icons (now there's font-awesome ) will be replaced with a custom icon font.
After this first phase, we'll deal with more structural improvements related to the elements such as forms, main navigation etc.
Our goal must be to make the Solidus product attractive also to the decision makers and to do so it is necessary to invest in the visual as well as marketing part.
Furthermore, a cured (as well as performed) product also gives the idea of solidity.
Some screenshots here: