Claro: Use less hard values in variables.less

[iTorrey]

LESS allows us to define colors using hex and operating on values using functions like darken() and lighten(). I played around with the variables.less for claro and tried to condense the amount of hex values used by defining @base-color which acts as the base color for the theme. Borders and hover states are then defined using darken(@base-color, 10%) etc.

This makes it possible to quickly roll a new theme by simply swapping out one or two hex values and recompiling which furthers our efforts towards making dijit themable through a theme roller type app.

Please consider my attachment for inclusion in the claro theme.

[bill]

Hi Torrey, that's a great idea.

The resulting colors are a bit different than before, not sure if a typical person would notice although I can see, for example, that the button border and hovered accordion border is a little darker than before. I'd like to hear if Jason and Julie are OK with that.

About the choice of master variables: When I look at claro I see blue and gray. You've made @base-color to be the blue, but I don't see a corresponding master variable for gray. MenuBar uses @bar-background-color, disabled buttons use @disabled-background-color, and unselected tabs get @unselected-background-color, etc. Do you want to add something to control all of them? Maybe we would have @primary-color and @secondary-color instead of @base-color?

[iTorrey]

Bill yeah the colors aren't perfect I can try to tweak and get them closer. As for primary and secondary I like that idea and will implement that and submit another variables.less

[iTorrey]

Updated the file. It's better but I can still work on it to make it perfect. What are your thoughts of it now?

[bill]

Turns out there are a number of issues with customizing claro's colors, specifically that some of the gradient images files used in claro contain blue, rather than just black / white / transparency. There's also blue in the image files for checkbox, radio button, animated progress bar, etc. So switching the theme's color is not so easy.

Marking this for 1.7, when we should look at using ​cross browser CSS gradients or image files without any blue color to make theme customization easier.

[Enzo]

I'm coding a claro theme shader tool, and I've seen several gradient files into claro that use the blue color of claro (forcing the shader to shade those files).

Would be much better of course to have a pure gradient with only black/white/transparency => do you think it's possible?

here are the files where I've seen claro color, are you sure the claro color needs to be hardcoded in all those files ? (just to be sure...)

• images/tooltipGradient.png
• form/images/button.png
• form/images/sliderHorizontal.png
• form/images/sliderVertical.png
• layout/images/splitterHorizontalHover.png
• layout/images/splitterVerticalHover.png

And last thing, in the 2 following files, there's 1 line nearly white but not white, the hexa is #F5F7FA

• form/images/sliderHorizontal.png
• form/images/sliderVertical.png

=> Do you think it's possible for those 2 files to have a pure white line, or playing with alpha channel if not a pure white is needed?

[bill]

I remember Jason saying that he needed the blue in the image files to get it to look exactly how he wanted, but I'd be willing to accept some change if it makes the theme more easily customizable.

If you have "patches" to those image files to just use black and white, or better yet can switch the theme to use ​CSS gradients rather than image files, I'd love to see it.

[ben hockey]

i've made an update to torrey's variables.less file. the differences are as follows:

• added @error-color
• changed @minor-selected-color, @base-border-color, @unfocused-clickable-color, @border-color, @pressed-background-color, @arrowbutton-background-color and @toolbar-combobutton-hovered-unhoveredsection-background-color to be a closer match to the current colors in the theme. it seems that due to rounding in the calculations, exact matches weren't always possible but the difference in the values used shouldn't be noticable.
• changed @disabled-text-color, @unselected-text-color, @focus-outline-color and @timepicker-minor-value-text-color to be derived from @secondary-color
• changed @hovered-background-color and @button-hovered-background-color to match current color in theme (@minor-selected-color)
• changed @nestedtab-hovered-border-color to @primary-color to match the current theme (see note below about hovered colors)
• changed @arrowbutton-inner-border-color to be @container-background-color
• changed @select-dropdownitem-hovered-background-color and @timepicker-value-hovered-background-color to @pressed-background-color to match the current theme (see note below about hovered colors)
• added in the missing mixins that have been added since this ticket was opened

NOTE about hovered colors:

torrey's variables.less file seems to try to make some of the hovered colors more consistent (by using more of the "base" hover variables) but my file simply tries to match the current theme. for the sake of consistency and simplicity it might be worthwhile to follow what torrey has done if the theme is not compromised by these changes.

this is obviously not perfect due to the images used in the theme but i think we should try to work towards adding this now and "fixing" the images when we can. at least this would be a step in the right direction. also, maybe we could move all "constants" (@document-text-color, @dnd-avatar..., etc) to the top of the file so that they are more obvious for anyone who is trying to build a theme by hand.

[ben hockey]

ben's attempt at variables.less

[bill]

I agree, feel free to check that in, and then we can make further improvements later.

I'm not sure whether Torrey was trying to make colors more consistent or just didn't know about this spin() function, but making the colors consistent sounds like a good idea. FYI, when I did the initial conversion to less I discovered lots of values that were very close, and it turned out it was just a mistake that they weren't identical. So maybe you are referring to more cases like that. Not sure.

[ben hockey]

In [26689]:

update variables.less to base the theme on a few colors and derive other colors using spin, darken, lighten, etc. refs #12306

[ben hockey]

i've been looking into adding variables for all the image references throughout claro as well. this should make it simpler for anyone who might create their own sprites, gradients, etc to update the theme to use their own images. i was wanting to get some direction about how to handle the following case.

css/less files in the root of the claro theme might reference an image in the forms directory as "form/images/foo.png" and that same image might be referenced from a css/less in the form directory as "images/foo.png". the best approach i've heard (as suggested to me by a colleague) is to have all files in the theme written in a way that assume all variables that reference images will be relative to variables.less.

as an example, TimePicker?.less and form/Common.less both reference the same commonFormArrows.png file in form/image/commonFormArrows.png (relative to the root of the claro theme). in TimePicker?, the reference is "form/image/commonFormArrows.png" but in form/Common the reference is "images/commonFormArrows.png". if i take the approach of referencing all images as being relative to variables.less then i would add @image-form-common-arrows: "form/images/commonFormArrows.png"; to variables.less and TimePicker?.less would reference @image-form-common-arrows and form/Common.less would reference "../@{image-form-common-arrows}" in order to adjust the path.

first, is there any interest in taking a patch that would extract all the image references into variables? if so, what do you think about this approach for handling relative references to the same file? is there an alternative that's preferred? i'm willing to get this patch completed today or tomorrow if there's an interest in it and i can get some direction about how to approach this issue.

[bill]

I'm not sure how many people add new image files in new locations, rather than just overwriting the old image files with their new ones. But anyway I'm fine with the change, sounds like it would help some people.

[ben hockey]

i have a case where the paths to the current images will get flattened and the names will be changed to UUIDs. this will be very helpful to me so i'll attach the patch when its completed. thanks.

[ben hockey]

patch to add vars for images

[ben hockey]

this patch pulls out all the image references as variables.

there is a downside where some urls have now lengthened - eg previously image urls in the form directory would use relative references to the images directory in the form directory: url("images/foo.png"). now, since there were a few cases where something in the top level directory would refer to an image in the form directory (or vice versa), i normalized all the url references to be relative to the top level so the example above becomes url("../form/images/foo.png") but ../form/ is technically not needed.

i just wanted to make sure this was clear before the patch gets applied.

[bill]

I did anticipate that downside. It's unfortunate, but not the end of the world. I'm OK w/it.

[ben hockey]

Replying to bill:

I did anticipate that downside. It's unfortunate, but not the end of the world. I'm OK w/it.

does this mean you're ok for me to commit the patch or do you want some more time to look at it?

[bill]

I'm fine with you committing it.

[ben hockey]

In [26820]:

committing patch that adds variables for image names in variables.less - refs #12306

[Enzo]

Just some questions on last commit: I can see there're in a standard claro theme:

• 13 images in form/images/
• 22 images in images/
• 9 images in layout/images/

However last commit only refer:

• to 11 images in form/images (why 2 images are missing?)
• to 21 images in images (why 1 image is missing?)
• to 0 images in layout/images (why layout images are missing?)

By referencing all images in variables.less, it would be possible to know if all images are used by claro or not (to delete zombie images)

[bill]

Ben - seems like after you address that comment above, this ticket can be closed, and marked as milestone 1.7. The gradient CSS should be a separate ticket anyway.

[ben hockey]

i completely overlooked layout. i don't know how that happened - sorry about that. i'll fix it today.

bill - if there are images that aren't used should i leave them or delete them? i would rather err on the side of caution and just leave them for now. given that i somehow managed to completely overlook layout, i'd hate to make the mistake of removing an image that was actually needed. maybe if we weren't at beta6 and knocking on the door of RC1 i might feel a little more comfortable but i'm trying not to be "the guy" that broke things at the last minute :)

[ben hockey]

In [26837]:

added variables for layout images. fixes #12306

[Enzo]

Ok thanks, So as a result, the 3 following files are missing in variables.less. But my rgrep doesn't find any of those files in dojo, dijit, dojox code (except one of them used in other templates than claro):

• images/treeExpand_loading.gif (used in tundra, soria and nihilo)
• form/images/button_grad_d.png
• form/images/shadow.png

=> Maybe could you consider deleting those 3 zombie images from claro directory?

[Adam Peller]

enzo_4: I suggest opening a new ticket

[Enzo]

ok Peller => #14119