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

v4 Color update #22551

Closed
wants to merge 26 commits into from
Closed

v4 Color update #22551

wants to merge 26 commits into from

Conversation

mdo
Copy link
Member

@mdo mdo commented May 1, 2017

While working on the forms update, I noticed we have quite a few ways of reassigning variables for our color system. The $brand- variables have been around for awhile as a support system for semantic naming, but honestly, it's an unhelpful abstraction. Not everyone's primary color is blue or their error state red.

This PR aims to drastically improve the power of our colors, simplify code by removing the $brand- and $state- variables, and increase the number of component variants and utility classes. This has been built in such a way that we can easily expand or add new maps for color shades after v4 ships. I went down a few paths the last couple weeks and ultimately decided this was the one of least resistance given a shorter window.

More specifically, this:

More still to come, including:

  • Finish updating Options docs
  • Revamp the colors to create an actual color scheme
  • Ideally improve contrast ratios
  • Consolidate and standardize variant names

See #18462 for first foray into color palette. That PR failed as it started with the brand and state colors vs abandoning them completely for a clearer and more customizable system.

mdo added 22 commits April 23, 2017 08:25
idea being we can nuke more component modifiers for these (e.g., card outline styles)
— fill in some colors for now with temp mixes
— make some basic palettes with utilities
— Scales back the plans for a full color system in v4.0.0 to instead build on a core set of colors. No shades or tints are coming just yet, but soon.

— Rearranges the variables file a bit to clean things up.

— Drops all `$brand-` color vars for `${color}-` vars for less abstraction in the colors setup. This differs from #18462 in that we don't reassign the `${color}` vars into a Sass map with different names; it's now `$red` and `map-get($colors,  red)`.

— Updates tables, list groups, alerts, form validation, and some docs CSS to use the new color variables.
@mdo mdo added this to the v4.0.0-beta milestone May 1, 2017
@include table-row-variant(green, $table-green-bg);
@include table-row-variant(teal, $table-teal-bg);
@include table-row-variant(orange, $table-orange-bg);
@include table-row-variant(red, $table-red-bg);

Choose a reason for hiding this comment

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

Color red should be written in hexadecimal form as #ff0000

@include table-row-variant(blue, $table-blue-bg);
@include table-row-variant(green, $table-green-bg);
@include table-row-variant(teal, $table-teal-bg);
@include table-row-variant(orange, $table-orange-bg);

Choose a reason for hiding this comment

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

Color orange should be written in hexadecimal form as #ffa500

@include table-row-variant(danger, $state-danger-bg);
@include table-row-variant(blue, $table-blue-bg);
@include table-row-variant(green, $table-green-bg);
@include table-row-variant(teal, $table-teal-bg);

Choose a reason for hiding this comment

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

Color teal should be written in hexadecimal form as #008080

@include table-row-variant(warning, $state-warning-bg);
@include table-row-variant(danger, $state-danger-bg);
@include table-row-variant(blue, $table-blue-bg);
@include table-row-variant(green, $table-green-bg);

Choose a reason for hiding this comment

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

Color green should be written in hexadecimal form as #008000

@include table-row-variant(info, $state-info-bg);
@include table-row-variant(warning, $state-warning-bg);
@include table-row-variant(danger, $state-danger-bg);
@include table-row-variant(blue, $table-blue-bg);

Choose a reason for hiding this comment

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

Color blue should be written in hexadecimal form as #0000ff

@include list-group-item-variant(green, $list-green-bg, $list-green-text);
@include list-group-item-variant(teal, $list-teal-bg, $list-teal-text);
@include list-group-item-variant(orange, $list-orange-bg, $list-orange-text);
@include list-group-item-variant(red, $list-red-bg, $list-red-text);

Choose a reason for hiding this comment

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

Color red should be written in hexadecimal form as #ff0000

@include list-group-item-variant(danger, $state-danger-bg, $state-danger-text);
@include list-group-item-variant(green, $list-green-bg, $list-green-text);
@include list-group-item-variant(teal, $list-teal-bg, $list-teal-text);
@include list-group-item-variant(orange, $list-orange-bg, $list-orange-text);

Choose a reason for hiding this comment

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

Color orange should be written in hexadecimal form as #ffa500

@include list-group-item-variant(warning, $state-warning-bg, $state-warning-text);
@include list-group-item-variant(danger, $state-danger-bg, $state-danger-text);
@include list-group-item-variant(green, $list-green-bg, $list-green-text);
@include list-group-item-variant(teal, $list-teal-bg, $list-teal-text);

Choose a reason for hiding this comment

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

Color teal should be written in hexadecimal form as #008080

@include list-group-item-variant(info, $state-info-bg, $state-info-text);
@include list-group-item-variant(warning, $state-warning-bg, $state-warning-text);
@include list-group-item-variant(danger, $state-danger-bg, $state-danger-text);
@include list-group-item-variant(green, $list-green-bg, $list-green-text);

Choose a reason for hiding this comment

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

Color green should be written in hexadecimal form as #008000

@patrickhlauke
Copy link
Member

In fact, I want to make all brand-success colors bright pink, so now all my success modifiers show bright pink backgrounds. This may suggest success to some people, but most expect success to be green?

so with the new model what would you do? override the color of .alert-green { color: DeepPink; }? is that better than redefining .alert-success { color: DeepPink; }?

@mabar
Copy link

mabar commented May 26, 2017

I would like to see in docs that 'success' is for successful operation and not for green. Writing one selector for different color on specific place is not much work.
But for cms developers this isn't good. Classes with -primary, -success etc. would be written everywhere. Much more code if we want our templates to be universal. And if anyone like to create new theme so that developer must know all our classes.
Moreover this solution is only partial. Eg. forms are highly coupled with specific variables. And it's absolutely not easily overwriteable as these classes are added by JS and by server. So we will must change our client and server scripts to use different colors? If we don't want to do that so in $green will be blue.

@bbmatt
Copy link

bbmatt commented May 26, 2017

so with the new model what would you do? override the color of .alert-green { color: DeepPink; }? is that better than redefining .alert-success { color: DeepPink; }?

No, I'd make my own modifiers as needed & frequently do.

So we will must change our client and server scripts to use different colors? If we don't want to do that so in $green will be blue.

I guess this is the danger of tying your CMS templates to a universal css framework. I completely get where you are coming from - this small change in bootstrap would result in a either a large change for your CMS. OR you could 'patch in' a CSS override, which would be very quick to do (re-introduce the classes you needed), 10 minute job.

The thing is, you can't expect an independent framework that is used across countless projects, to ensure backward compatibility with your own project. On the flipside of that, you wouldn't want to rely on a framework that constantly changes the rules. There's a balance to be struck.

@patrickhlauke
Copy link
Member

No, I'd make my own modifiers as needed & frequently do.

so the change (or no change) to the core BS classnames wouldn't actually affect you then. so no problem keeping them as they are now, which seems to make more sense for other devs, at least based on the conversation here so far?

@brianteeman
Copy link

This is NOTHING to do with backwards compatibility at all.
This is about ensuring that the majority of bootstrap users are able to continue using bootstrap

@bbmatt
Copy link

bbmatt commented May 26, 2017

For people who vehemently defend the idea that class names don't matter and there's no semantic value in them (to developers)...in that case, it wouldn't make a difference if we kept the current class names rather than changing them to use color names, no? Since they can be arbitrary, there's no harm either way...

Class names do matter, of course they do. I would never deny that. In this case, we have a parent class & we have a modifier. What is being proposed here is to change a very prescriptive set of modifier names to something far more generic - color. What else would you use that isn't prescriptive in terms of meaning? As soon as you start tying more emotive nouns to classes, you kinda limit their semantic use for coders.

In order to change the colors in the framework, a coder would be expected to know how to do this and how to use preprocessing. If they wanted to make btn-green purple, surely they could just as easily create a btn-purple modifier? Or maybe btn-signup?

With the existing classes, sometimes it just feels plain wrong to use the classes in terms of viewing the code.
Perhaps I have a strip of buttons that perform operations on a list of items. I want to make them blue. With the current implementation, each of those buttons would have a modifier of btn-info. But these buttons are not info buttons. They perform specific tasks - perhaps one is an edit button, another may be a move button. They have different use cases. They only thing I do know, is I want them to all be a specific color.

@mabar
Copy link

mabar commented May 26, 2017

Re-using code is what every developer should want. Eg. login form can be used without changes for eg. 10 websites. And when I have 10 themes on all of them so 100 modifications are needed to write? When I write only one before? Annoying. For cms developers and for developers using this css too.

My cms is nout bound to bootstrap. But is used as default. And default should be easy. No-one will continue with building complex systems with bootstrap. Large systems as joomla are depending on reconfiguring bootstrap is only what is required to change look. Rewriting html, php and js to change look is simply stupid.

Every framework developer in any language would say abstraction is better.

@bbmatt
Copy link

bbmatt commented May 26, 2017

Every framework developer in any language would say abstraction is better.

In some circumstances, sure, but in this case, the abstraction has meaning that may not be wanted!
It isn't abstract enough. It prescribes the nouns success, warning, danger and info whether you want them or not!
Also, that's a very generic statement - there are many many many cases where you want to be very specific with your use of variables, modifiers, methods or classes. :)

@mabar
Copy link

mabar commented May 26, 2017

Think in context of all used languages. Biggest problem are forms. Not buttons. In current state you can use parrent button class and apply modifier in same way as with this PR. If I'll forget it's problematic in cms to replace primary with green etc. so problem is still the same. Forms are highly coupled with specific colors. Everyone must reinvent wheel. And developers which come to this code must learn, how was wheel reinvented.
And if current abstraction is not good enought - it should be improved. Not removed.

@blikblum
Copy link

In order to change the colors in the framework, a coder would be expected to know how to do this and how to use preprocessing. If they wanted to make btn-green purple, surely they could just as easily create a btn-purple modifier? Or maybe btn-signup?

This is not how themes with color variants work.

Example: You have a button with class btn-success that by default is blue

The theme also provides a variant where buttons with such class have pink color. This is possible without changing the markup, just changing to a CSS file where btn-success has a pink style

With the proposal the button would have a btn-blue class by default. To provide a pink color variant that does not require markup change the theme author would need to create a CSS rule where btn-blue have pink style.

The theme author could create your own semantic classes with the drawback of increasing the cognitive load of theme user that has to learn new markup instead of using his BS knowledge.

Also if this will require at least theme and CMS authors to create your own rules where is the usefullnes​ of it?

BTW: look at wrapbootstrap and themeforest to see how important is theme market for BS users

@mbabker
Copy link

mbabker commented May 26, 2017

I honestly feel like this change favors developers and custom theme designers over organizations that are distributing reusable themes or platforms like Joomla where Bootstrap was standardized as the default UI framework (a decision which helped to drive a fair majority of our templating ecosystem to standardize on Bootstrap as well). Sure, for those of us with the savvy and skill to make the changes, editing some HTML to change blue to green may be easy, or for theme distributors adding a "middle layer" on top of Bootstrap to retain a set of more semantic (and therefore generic and reusable) can be done, but this hurts end users in a space where they aren't as technically savvy and are either going to be the ones hacking their CSS to .blue { color: green; } or it will be harder for them in the case of a CMS environment to keep their template up-to-date with the parent source because they will have to make an excessive number of changes to change a color attribute.

As @wilsonge hinted to, and as I'm now saying as Joomla's current production lead, we are watching this with a vested interest as this change could require us to re-evaluate our UI defaults.

@bbmatt
Copy link

bbmatt commented May 26, 2017

drawback of increasing the cognitive load of theme user that has to learn new markup instead of using his BS knowledge.

I think we should give credit for people having enough curiosity and ability to understand a minor amend to modifier classes!

Also if this will require at least theme and CMS authors to create your own rules where is the usefullnes​ of it?

Experimentation, expression, customisation, expandability, learning. Using a framework and extending on it to suit different needs?

This is one of those discussions that I guess is purely a matter of personal choice. I see this proposed change as being beneficial - getting rid of prescriptive modifier class descriptions. Others see it as a terrible crime against css, daring to use color names in modifier classes - shock! horror! Others see it as something that will result in changes to their own projects.

@ghost
Copy link

ghost commented May 26, 2017

Just get on with it, or Foundation beckons...

@patrickhlauke
Copy link
Member

feeling i'm getting the more i hear the discussion:

  • this change will benefit authors who used the warning, success, etc modifiers not for their meaning, but to achieve a certain look (e.g. using success because it's green, not because it indicates success - rather than overriding the color of whatever more appropriate/meaningful modifier class name in their own additional styles)
  • this change will require theme authors and people wanting to easily reskin/change the default theme to modify their HTML output to use different classes / assign new classnames and compile their own bootstrap styles

so there are two sides, both to some extent with merit. i still heavily lean towards not changing the classnames. @mdo, after throwing this hand grenade of a PR...any thoughts? because this has the potential of turning into a never-ending discussion otherwise ;)

@brianteeman
Copy link

And it will disadvantage any cms who used the warning, success, etc modifiers for their meaning

@browner12
Copy link
Contributor

I agree with you @patrickhlauke, there is definitely merit to both sides. For me as a developer, my mindset was always exactly what you listed. I never thought, "I need a success button", I thought "I need a green button", so the semantic class names lost their true meaning.

Is the simple solution to include both options? Make the changes to allow developers to have more flexibility, and retain the semantic names for the theme developers? Does this cause too much bloat, or is there some other downside to this?

@blikblum
Copy link

drawback of increasing the cognitive load of theme user that has to learn new markup instead of using his BS knowledge.

I think we should give credit for people having enough curiosity and ability to understand a minor amend to modifier classes!

Is not about curiosity or ability to change the markup. In an practical example all admin templates i've found (admin lte, propeller) uses alert-danger, alert-success etc to style alerts. When start using such themes those familiar with BS already know how use them. There are thousand of users of such components, see the sell stats in market places

With the change each theme author can create it own class name abstraction, e.g. alert-confirm, alert-note, alert-error, others may recreate the old classes names and finally there will be those that will use alert-green, alert-blue.

This is one of those discussions that I guess is purely a matter of personal choice. I see this proposed change as being beneficial - getting rid of prescriptive modifier class descriptions

Is not about personal changes, in this discussion there are tons of technical arguments.

Also, about being prescriptive this is IMO one of the Bootstrap selling points. When i was evaluating a CSS framework i looked Foundation but, although technically good, it does not provide ready to use components to create a UI. With Bootstrap + custom themes i can leverage a good looking UI without the need to implement my own unique design from scratch. I believe a significant part of BS users fall in this category.

@bbmatt
Copy link

bbmatt commented May 29, 2017

Reading through this thread, it struck me that obviously there's a lot of self-interest here. Theme developers & users, CMS developers. Absolutely nothing wrong with that - these changes would incur a fair amount of change.

I had a quick look through the propeller admin theme that blikblum linked too. Sublime text reports 77 entries for -success. Not all of those have been removed in this proposed PR (has-success for instance). So sure, lots of amends to CSS and HTML.

However, the glaring elephant in the room for these themes & CMS's, is that the move to Bootstrap 4 is going to incur far more than just the proposals this particular PR would create.

https://github.com/twbs/bootstrap/blob/4a8728d54c2a89b923ca019f8daf497927aeccdd/docs/migration.md

Boom. The migration.md document.

The changes here are far more sweeping than dropping a few modifier classes.
Developers of themes & CMS's systems and countless thousands of websites built with bootstrap will need to make a judgement call will need to be made whether to attempt to "patch in" the changes or to rewrite. That depends how modular the implementation of bootstrap is in each instance. There's also the option to just stick with bootstrap 3 whilst making migration plans.

To return back to the propellor theme as an example, this theme has implemented it's own card solution, pmd-card. So the developers have to decide whether to keep this or to move over to the bootstrap 4 card when they migrate.

That is just one migration issue the theme has to face.

So really, the talk about how this minor amend to modifier classes impacts these themes and CMS's doesn't amount to a whole lot when panels, wells & thumbnails are replaced by cards, glyphicons are gone and numerous other changes have happened.

We're still left with the semantics of the modifier class, prescriptive nouns vs color.

... waits for the inevitable downvotes ... ;)

@brianteeman
Copy link

@bbmatt Sorry but you are still not understanding the issue. All the b/c changes such as panels, wells and thumbnaills are one time changes to the theme to make the theme compatible with bs4. The proposal under discussion here will effect every theme for all time as stated by multiple people multiple times

@bbmatt
Copy link

bbmatt commented May 29, 2017

I completely understand the issue and really don't think it's a big deal at all. These are just modifier classes. That's it. Sure, people have learned these patterns and have come to expect they are in place, but they will also have used panels, wells and icons.

@patrickhlauke
Copy link
Member

@bbmatt i think you made your point. others have also made their point that they'd prefer modifier classes that aren't tied to a color name. i guess we'll have to agree to disagree here instead of going around the same circular discussion.

@mabar
Copy link

mabar commented May 29, 2017

As Patrick said, it's not about just changing few modifiers when upgrading to bs4. It's about changing them everytime, when colors are changed. And not only in css but in html too. It means more work.
More work for theme developers, cms developers and drag'n'drop builder developers.
Unfortunately for you @bbmatt this change would affect negatively more bs users then positively. Bad idea to do for the purpose of little improvement in intelligibility of markup. Moreover it's improvement mainly for developers who just click on download button. And what will do that kind of developers with .button-green when want color which is not in defaults? !important override

@bbmatt
Copy link

bbmatt commented May 30, 2017

And what will do that kind of developers with .button-green when want color which is not in defaults? !important override

Does a kitten die each time someone does that? No. Seriously, why do some devs get so hung up on this, like it's some cast in iron rule that "thou shalt not use color names in css modifier classes".
So what if someone slaps !important and overrides alert-green with red?
What will happen? Nothing. It will make their code make less sense, that's it. Big whoop.

@twbs twbs locked and limited conversation to collaborators May 30, 2017
@mdo
Copy link
Member Author

mdo commented May 30, 2017

I probably should've locked this earlier, but doing so now. I'll be revisiting this PR shortly.

@mdo
Copy link
Member Author

mdo commented Jun 15, 2017

Picking this up in a fresh PR that focuses on ensuring theme-ability via CSS and Sass by preserving a "brand theme" while also adding more options for colors. See #22836.

@mdo mdo closed this Jun 15, 2017
@mdo mdo deleted the colors branch October 31, 2017 21:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.