Simplify the design of Chosen, and organize the CSS

[mlettini]

@pfiller @tjschuck @koenpunt

This is a very large and significant design/css change. It most definitely breaks currently open PRs that touch CSS. It's probably the biggest since the last time we updated the design of Chosen.

Background

The design of Chosen is currently quite dated. Single Chosen's gradient (also used on Multi Chosen search-choices) is based off of OS X select boxes, but those have been redesigned as of Mavericks. The design of both Single and Multi are not terribly consistent, which causes extra and unnecessary CSS to accommodate. At the same time, the repo's Sass has been growing cruft. Properties are out of order and inconsistent across the Sass file.

This has been bugging me for quite awhile. When I come across Chosen in the wild, it sticks out like a sore thumb, since it doesn't fit in with the design around it. Or the developer had to override a ton of properties just to have it fit into their app/website. It'd be nice to work Chosen towards a possible "theme-able" future.

Doing something about it was a slightly overwhelming task, considering how much Chosen is used, and how many open PRs would most likely break. We had a hackweek at Harvest last week, and I decided to give it a shot. This might be shot down because of it's size and the number of open PRs, but I do believe it is a positive step forward for Chosen. It could be considered a 2.0 (from a design-only perspective, anyway).

What's Changed

• The design of both Single and Multi Chosen has been updated to be simpler and more consistent
• All gradients have been removed (should allow Chosen to fit in better with both PC and Mac)
• Added the outline property so Chosen's focus design fits into the standard OS X protocol (we could make this an option, but that requires JS)
• Started using variables for sizes and colors (this is the beginning of a possibility to allow theming of Chosen)
• Alphabetized all CSS properties (we should make this a rule, so as more changes happen, the code stays easy-to-read for everybody)
• Organized the Sass file, removing unused definitions and properties (there was stuff in here from the very first design)

I will comment directly in the code below pointing out anything helpful. Because of the alphabetizing of properties, I imagine the diff will be a little hard to read. Rather than post a bunch of screenshots, you should pull and look at this version side-by-side with the current version.

[mlettini]

1. The <b> </b> was unnecessary, as I've moved the sprite image right onto that parent <div> </div>.
2. I've added a placeholder of "Filter" to this input. Watching people use Chosen in Harvest we've seen them skip right over the input. We should probably make this customizable, but maybe in another PR.

[stof]

The placeholder needs to be configurable through an option, to allow people to translate it (all other texts are configurable)

[koenpunt]

This change should be applied to the prototype version too

[mlettini]

👍 I'm going to remove the placeholder for now. I think this is a feature/option we should add to Chosen, but I'll keep this PR focused on the design changes.

[koenpunt]

I took a quick look and think this is a step in the right direction. For now I only wonder why sort properties alphabetical? Because it doesn't define or visualize the relation between them.
I prefer sorting them by category; display, positioning, fonts, colors, etc. Although I can't say that I'm very strict on this ..

[mlettini]

Thanks @koenpunt - I'll add that !default flag to the variables.

Why we should alphabetize CSS: The main reason is that this is an order that anyone can understand, read, and contribute to. There is no set "proper" way to write CSS in today's landscape, which means designers/developers all make their own. As you mention, you have a preferred sorting method by category. Another developer using this repo and reading the code might have a different preferred sorting method, using similar categories but a different order of them. And both of you could write your properties in different order per category... like you could keep your width and height as part of the "display" properties, and another developer likes to add them with "positioning".

When I read the current Chosen code, I feel like I had to read the entire property list on a class to understand how it behaved. It was hard for me to find if this class had a position: property on it, since that property could be first in the list, or last, or somewhere in the middle. And it's order location was not consistent across classes.

By writing CSS in alphabetical order—especially for an open-source library that will be read by many people—it should help developers reading the CSS find what they are looking for with less hassle. And it keeps property order consistent across the whole file, thus making the library easier to work with.

[tjschuck]

I agree that if this goes in, it should be for a 2.0 release.

That said, we should definitely release a final 1.x series release before this goes in -- there are a lot of commits on master that haven't been rolled into a release version yet. (See the current draft notes on the releases page).

[stof]

@mlettini I'm with @koenpunt regarding the property sorting: with alphabetical order, you still have to read the entire list to understand what it does, because alphabetical order is unrelated to the effect of properties.
A better way to ensure the order stays consistent would be to put a csscomb config file in the repo, and run it on Travis. Bootstrap uses csscomb to ensure the order: https://github.com/twbs/bootstrap/blob/master/less/.csscomb.json

[koenpunt]

@mlettini I remember you being opposed to use a tool like CSSComb for sensible sorting? My main concern with alphabetical is that properties like width and height can be miles apart.

[koenpunt]

In comparison:

http://csscomb.com/online

[mlettini]

Personally, I find that code on the right to be hard to read. While yes, properties that are semi-related to each other are closer to each other, it doesn't promote ease of reading. If I wanted to update the box-shadow, I'd have to scan the entire property list to find it. It's weird that it's at the bottom of the list, when it starts with "b", and I usually look for words that start with "b" at the top of the list. The functional relationships are not perfect either: if not using box-sizing: border-box, then a border would affect the box-model and should be near the padding. Sometimes line-height defines the height of an element if height is not explicitly set. outline has more to do with :focus styles than with borders. Also the spaces you might add to to keep related things together are not there when you Inspect Element in the browser (another area where reading CSS should make sense).

At Harvest, we've been using alphabetical order for all CSS for a few years now, and it's done a good job of keeping our CSS (across all repos and many files) easy to use, easy to maintain, and consistent when many people are in the code. I'm curious how you would feel towards alphabetically ordered CSS properties if you were to try it for a bit. Either way, this conversation gets to the root of the problem: different developers have a different pre-conceived notion of how property order should be set, and bring that notion with them when reading other CSS files. That makes me prefer "ease of reading" over anything else when I know other people will be reading my code, and I have to read their code.

(plus, aside from height/width, most related properties are quite near each other in alphabetical order already: background/border, font-stuffs, margin/padding)

---

Now, all that said, I'd be fine with Chosen using CSSComb. Chosen is just one file, and by using a tool to set the property orders, we'd at least still be maintaining consistency across the file. We should, though, ensure this stays the same with all future changes, by making the combing happen automatically.

---

Aside: @tjschuck noted to me that this change will (unfortunately) mess with the CSS file's history, rendering Blame almost unusable. Adding/using CSSComb would do the same. I think this design step forward is still worth that loss.

[koenpunt]

About the sprite, do we need 2 of them? Can't we just use the retina version on low dpi screens as well?

[mlettini]

@koenpunt I began working on Chosen with only 1 image, the retina one, but the issue with that is the 2x sprite requires background-size. That is not supported in IE8, and Chosen currently has legacy support for IE8. I'm not entirely sure if we should proceed with that now. Thoughts?

[koenpunt]

Hm yeah good point, then the question is; for how long do we want to support IE8? Because we're probably not going to drop IE8 support because of the sprite.

[mlettini]

@koenpunt

for how long do we want to support IE8?

Probably a conversation for another time 😄

And we're probably not going to drop IE8 support because of the sprite.

Agreed.

I'll have a new version available soon. There was a small conversation outside of this PR that the single chosen doesn't look clickable / doesn't stand out / doesn't look like a select box (compared to the current Chosen). So I'm toying with the design a bit for a nice middleground.

[koenpunt]

Looks good!

We might consider checking how this would end up looking in a Bootstrap template?

[koenpunt]

Just noticed that the color difference between disabled and selected options is minimal:

[mlettini]

@koenpunt I've improved a few things you see in my latest commits, including difference between disabled and selected in multi (note current Chosen just makes these look the same).

I pulled Bootstrap in and Chosen still looks the same, so successfully not allowing any of the design to be over-written. This PR only lands a foundation for theming (with variables and some other improvements), but thats still a goal for a future PR. It probably involves more than just colors, as Chosen should work well with different font-sizing and padding.

So in order for this PR to move forward, we need:

• Another Chosen release (as mentioned by @tjschuck Simplify the design of Chosen, and organize the CSS #2479 (comment))
• Merge this
• Add in CSSComb to config file, so it runs automatically (as mentioned by @stof Simplify the design of Chosen, and organize the CSS #2479 (comment))

Do I have that correct?

[koenpunt]

I pulled Bootstrap in and Chosen still looks the same, so successfully not allowing any of the design to be over-written.

What I meant with my bootstrap comment is –because lots of people use Chosen within a bootstrap styled website– that it would be a pro if it does fit in with the bootstrap style.

[koenpunt]

And I still think the difference between selected and disabled is minimal. What if we add a grey background, just like a normal multiple select:

[mlettini]

What I meant with my bootstrap comment is –because lots of people use Chosen within a bootstrap styled website– that it would be a pro if it does fit in with the bootstrap style.

I agree! I think this Chosen design works well with Bootstrap as it is, even if it's not perfectly Bootstrappy (the padding's aren't the same for inputs, single chosen has a gradient while Bootstrap is fairly flat, etc). This design works better than current Chosen.

What I was saying is that Chosen could fit in more with Bootstrap, but I think that should be left to a bootstrap-chosen theme (once we work on better theming). This lays the groundwork for that at least. Do you have other design suggestions that could work for default Chosen to help it fit more into the Bootstrap style while also being a good default for anyone? All ears.

What if we add a grey background, just like a normal multiple select:

A default multi-select is different than multi-chosen. Multi-selects need to highlight what you've selected because otherwise there's no other indication for you to know! Chosen, however, highlights what you've already selected both with the selections at the top, and the checkmark in the dropdown. This dropdown is also only shown when you go to select more options, so during this interaction I think we should not be highlighting what you've already selected more, because that grabs your attention away from what you haven't selected.

[koenpunt]

Multi-selects need to highlight what you've selected because otherwise there's no other indication for you to know!

Hm, good point. But still the hint that an option is selected is quite far from the content of the option.

And is the background color for the no results message intentional?

[mlettini]

What are your thoughts on italicized selections? Edit: scratch that

And is the background color for the no results message intentional?

Yes, for the same (but opposite) reason we are just talking about. When you are typing and get down to "nothing found", we want the user to realize this. The background helps it stand out, otherwise it just looks like an option.

However, now that I'm looking at it, it makes the "nothing found" look like it's selected. Might re-think that...

[mlettini]

@koenpunt Removed the background-color on .no-results - what I found was that if you're typing something out and that pops up with a background, it looks selected with a background color, and my brain instinctively wants to just hit "enter" to select without reading. By removing that, it doesn't look selected. Tried the same thing for .result-selected (as you suggested above), and the same thing happens: when I filter down to a result that is already selected, if it has a background color I think it is highlighted and I can select it (when I cannot in multi-chosen). So that's another reason I do not think .result-selected should have a background color.

[boynet]

+1 to @mlettini the normal multiple select has bad ui, so we don't need to copy its behavior if it was good we didn't need chosen( @microsoft )

[mlettini]

I haven't looked at this in awhile and getting back on it. @koenpunt I simplified the design by removing the gradient and box-shadow I had on there. This makes it look better in the Bootstrap styling, and also allows someone grabbing and using Chosen to more-easily customize it to fit their needs (an effort also started with the variables). It will need a bit more effort for real theming (so we can have an actual Bootstrap theme), but I don't think that will be too difficult later on.

I've also made a couple more adjustments, improving disabled support, and removing unnecessary compass includes. The only one left is the box-sizing include, and I suspect that can go away too (but wish to do so in another PR). The next steps for this PR, I believe:

• I think @pfiller is going to create the next Chosen release, v1.4.3, with the currently merged fixes (mentioned by @tjschuck Simplify the design of Chosen, and organize the CSS #2479 (comment))
• This will get merged in pending a final +1 to the code (should I open a new/clean PR?), and will be part of a 2.0 release
• Someone can add CSSComb or other tool for automatic sorting on build (as mentioned by @stof Simplify the design of Chosen, and organize the CSS #2479 (comment)), and perhaps there are other open PRs that should get into a 2.0 release

[Altreus]

It was suggested I mention here that relative units would be better than pixel values. When we incorporated Chosen into our recent project, the controls and text all seemed relatively small, and scaling it up, while not awkward, could have been avoided if Chosen's sizing were based on its container's.

The doubling of the PNG certainly helps to alleviate the other problem, which was that I ended up styling the controls to use FontAwesome so it scaled up beyond the native size of the PNG.

[mlettini]

@Altreus I think that's a 👍 idea. Although to not make this specific PR any larger than it is, I suggest that work be done separately after this (eventually) gets merged.

I think the best course for Chosen specifically is to use em's throughout, with one pixel-based font-size on the parent element. Using rem's could have unintended consequences depending on the the font-size applied to the HTML element in any project Chosen is added to (could be too big or too small). Using em's gives more flexibility.

[mlettini]

It's been on my list (for quite awhile) to do something with this PR. Obviously it's grown quite old, and I'm closing it, since in Chosen's latest release we announce that we're done with new features until further notice.

To preserve the work I did (if anyone's interested), I've forked the repository: https://github.com/mlettini/chosen/releases/tag/v2.0

👏