search

dojo / dijit-themes

Dojo 1 - modern themes
Other
29 stars 21 forks source link

flat theme updates #9

Closed btfou closed 8 years ago

btfou commented 9 years ago
wkeese commented 9 years ago

These changes seem reasonable to me.

BTW do you guys really want to check in the generated CSS files? I agree that "bower install themes" (or however you get a built version of this code) should have the generated CSS files, but I wonder if github should have them. The only reason dijit has the generated files is for historical reasons.

I also noticed two issues unrelated to these commits:

(1) In the color picker (both small and large), the choice for white looks weird because it's white-on-white with no border. Compare claro:

claro

with the flat color picker:

flat

(2) The arrow directions on closed TitlePanes seems weird to me. Typically it's a right arrow for a closed pane and a down arrow for an open pane. Anyway, that's a matter of preference.

kfranqueiro commented 9 years ago

RE CSS in the repo, bower doesn't have a separate repository/storage - github is its repository, so either the CSS needs to be committed here or there would need to be a build process to generate something to be committed to a separate repository. (Though I suspect Dylan is interested in this getting built into the Dojo 1.11 release, anyway.)

Should those two issues be entered separately?

wkeese commented 9 years ago

Oh you are right, I forgot that bower pulls from github. Still seems strange to check in built code into the main github repository, but I can't think of a better solution offhand.

@btfou, feel free to enter those other issues separately if you agree they are issues but don't feel like fixing them right now.

wkeese commented 9 years ago

PS: I can see why you don't want a border around the color squares; perhaps you should just have a border around white.

btfou commented 9 years ago

@wkeese I was looking at the TitlePane/Accordion pane arrows and agree closed should be right arrow. I suspect it may have been to avoid extra class for RTL. I'm going to change that up when I get back to this. This PR or next.

dylans commented 8 years ago

Landed what has been finished so far in master as cfab57c . @btfou will leave your next round of iterations for another PR. Thanks!