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

ui: Palette Cleansing #16550

Merged
merged 1 commit into from
Jun 19, 2017
Merged

Conversation

mrtracy
Copy link
Contributor

@mrtracy mrtracy commented Jun 15, 2017

Cuts down our stylus color palette to a small fraction of the colors
defined previously; early in UI development we were rampantly defining
colors with little thought given to any sort of common palette, and
there was a lot of cleanup debt remaining. As of this commit, we are
using only 21 colors in the UI, and each one of them has a consistent
semantic meaning.

Special thanks to @kuanluo for sitting with me for two hours while we
tediously went through this.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@mrtracy mrtracy requested review from tamird and BramGruneir June 15, 2017 22:48
@tamird
Copy link
Contributor

tamird commented Jun 16, 2017

:lgtm:


Reviewed 19 of 19 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


pkg/ui/styl/pages/debug.styl, line 34 at r1 (raw file):

  &__row
    alternating-background $table-border-color $table-border-color

why the repetition?


Comments from Reviewable

@BramGruneir
Copy link
Member

:lgtm:


Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


pkg/ui/styl/pages/debug.styl, line 34 at r1 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

why the repetition?

Yeah, this should have two different colors.


Comments from Reviewable

Cuts down our stylus color palette to a small fraction of the colors
defined previously; early in UI development we were rampantly defining
colors with little thought given to any sort of common palette, and
there was a lot of cleanup debt remaining. As of this commit, we are
using only 21 colors in the UI, and each one of them has a consistent
semantic meaning.

Special thanks to @kuanluo for sitting with me for two hours while we
tediously went through this.
@mrtracy mrtracy force-pushed the mtracy/ui-palette-cleanse branch from 31204f4 to 9c4fda6 Compare June 19, 2017 06:17
@mrtracy
Copy link
Contributor Author

mrtracy commented Jun 19, 2017

Review status: 17 of 19 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


pkg/ui/styl/pages/debug.styl, line 34 at r1 (raw file):

Previously, BramGruneir (Bram Gruneir) wrote…

Yeah, this should have two different colors.

Fixed, using white for now; these should be getting a design pass soon anyway.


Comments from Reviewable

@mrtracy mrtracy merged commit f7b7398 into cockroachdb:master Jun 19, 2017
@mrtracy mrtracy deleted the mtracy/ui-palette-cleanse branch June 19, 2017 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants