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

Recolor admin nav #603

Merged
merged 5 commits into from
Jan 14, 2016
Merged

Recolor admin nav #603

merged 5 commits into from
Jan 14, 2016

Conversation

Sinetheta
Copy link
Contributor

This is the first step towards Solidus admin branding

screen shot 2015-12-18 at 2 08 19 pm

background: $color-sidebar-bg;
z-index: $z-index-navbar-flyout;
}

.admin-nav-menu {
text-transform: uppercase;
text-transform: capitalize;
Copy link
Member

Choose a reason for hiding this comment

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

Please don't use capitalize here, because that changes how every single word looks on the admin tabs. Especially plugin maintainers and users of foreign languages may not want to have every single word capitalized. This should be handled inside the locale files.

Copy link
Member

Choose a reason for hiding this comment

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

As you can see perfectly on "Go Back To Store" ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ha! Good point. I think I wanted uppercase gone, and saw capitalize as the next step down. Let's be rid of it entirely!

@cbrunsdon
Copy link
Contributor

@Sinetheta this looks really nifty. And pretty. Etc.

Can you please do a write-up either in the wiki or somewhere on how one would go about customizing the colors themselves? I imagine a few stores will either want to immediate take use of the better organization to color to their own branding, or maybe even go back to what it was to avoid scaring people.

@tvdeyen
Copy link
Member

tvdeyen commented Dec 19, 2015

The new styleguide would be a great place to add the customization guide, isn't it?

As the colors now differ from the main content, I guess you will address this as next? Otherwise the admin UI will look a little weird.

Although I spoke against the dark/blue theme, I have to admit that it looks nice and far more mature then the current theme and I can't wait to see the whole backend to look like this! As always great work @Sinetheta 👑

@Sinetheta Sinetheta force-pushed the admin-colors branch 5 times, most recently from 71f1c8f to 8224395 Compare December 22, 2015 21:21
@Sinetheta
Copy link
Contributor Author

@cbrunsdon I've moved things around so that the "theme" of the sidebar is controlled by the final commit, a change of variables only, then added a note to the changelog.

I don't think any additional explanation is required for customizing the nav, as that file would be the point of entry for any solidus admin custom colors.

@tvdeyen I very much agree about noting these colors in the styleguide. I will update whichever PR makes it in later #602 (comment)

@cbrunsdon
Copy link
Contributor

Okay, as per discussion in the #gui room in slack (which we made to talk about the gui) and #630, we're leaning towards pulling out the last commit here (that actually changes the colors) for a future PR when more colors are ready to change. Final decisions still pending though.

@tvdeyen
Copy link
Member

tvdeyen commented Dec 30, 2015

👍

@Sinetheta Sinetheta force-pushed the admin-colors branch 2 times, most recently from 48ff9a8 to 4542081 Compare January 6, 2016 17:56
@Sinetheta
Copy link
Contributor Author

Hey guys, sorry I was away for so long. Although I personally love incremental changes in UIs, even if it results in half-man half-monster in-between phases, I can completely understand @tvdeyen's concerns. As such I have ammended the final commit to be opt-in instead of opt-out for users.

+    * Added optional "Solidus next" styles to the admin area, as per [admin rebrand](https://github.com/solidusio/solidus/issues/520).
+    To use the new colors, add `@import 'spree/backend/next/globals/variables_override';`
+    to your `spree/backend/globals/variables_override`.

Does that seem like a reasonable approach to everyone? The /next directory would be where we add any breaking style changes during rebrand until they can all be moved into core. For this PR I was able to get away with variable overrides only, but I wanted to leave the door open to custom stylesheets which a user could include after Spree's styles.

@jhawthorn
Copy link
Contributor

I like this as a way to get this in sooner, rather than make Kevin rebase weekly (which this PR needs again 😉)

We had a quick discussion IRL, and I would prefer we made a "theme" folder for this and give it a name. This would allow us to make a themes/classic_overrides.scss file to restore the "old" (current) colours when this theme becomes the default.

@tvdeyen
Copy link
Member

tvdeyen commented Jan 6, 2016

I like the idea of a theme folder, but it's not necessary right now IMO.

Although I would call the intermediate folder solidus I'm 👍

@Sinetheta
Copy link
Contributor Author

That's wise @jhawthorn because every time I have to rebase a branch my file names become more ridiculous.

@jhawthorn jhawthorn removed this from the 1.2.0 milestone Jan 6, 2016
@jhawthorn jhawthorn added UI changelog:solidus_backend Changes to the solidus_backend gem labels Jan 6, 2016
@jhawthorn
Copy link
Contributor

With the theme change I am now 👍 (other than one comment) since this doesn't change the default styles

@Sinetheta
Copy link
Contributor Author

Thanks for spotting that one @jhawthorn. I think we're good now.

@cbrunsdon
Copy link
Contributor

this is good by me now 👍. For the record though, we don't need to call the variable settings in every theme variable_overrides, as those in fact are the variables for that theme.

The new dark theme is not going to have aborder at all, so a color
variable isn't sufficient, we need to be able ot remove the
border entirely from variables.scss
We want to be able to control this area from variables.scss as well.
There were still some inconsistencies in the navbar variables.

Now we can swap color schemes as a single commit.
For increased contrast between nav and content.

We're also going to try to get away from the "numbered colors"
with our sass variables.
@cbrunsdon
Copy link
Contributor

Okay! bit of an odyssey to get this in, thanks for the patience @Sinetheta, great step forward

cbrunsdon added a commit that referenced this pull request Jan 14, 2016
@cbrunsdon cbrunsdon merged commit f353e7e into solidusio:master Jan 14, 2016
@ajkamel
Copy link
Contributor

ajkamel commented Jan 28, 2016

@tvdeyen @Sinetheta I found the same issue with using the blue theme similar to @adaddeo.

I found that manually overriding the spree_admin.scss file allowed me to import the blue theme however if I do not have that file it will not work. Any ideas why?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:solidus_backend Changes to the solidus_backend gem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants