Page MenuHomePhabricator

Variable naming in WikimediaUI Base
Closed, ResolvedPublic

Description

As WikimediaUI Base is becoming a public interface (still early stage, little dependencies), I'd like to discuss early-on several variables naming questions, as naming is

  • hard,
  • hard for a non-native English speaker like myself,
  • hard to change later on if insufficiently clear

There are several ideas and questions that I'd like to have clarified:

  • --color-base-contrast: contrast was meant to describe the opposite to the normal color combination, which after thinking about it reccurringly seems ambigous. -invert makes probably more sense, although it doesn't describe the inverted color of -base (#222) fully correct
  • *-filled-disabled: Is this understandable for a native speaker. If the widget is filled disabled, that's the color or border-color for it.
  • I'd like to change the modifier variables of a base variable to use -- in order to be easier readable (in the tradition of BEM's two dashes style) and featuring self-documenting code

So instead of

/* Foreground Colors */
--color-base:                 #222;
--color-base-hover:           #444;
--color-base-active:          #000;
--color-base-contrast:        #fff;
--color-base-emphasized:      var( --color-base-active );
--color-base-disabled:        #72777d; /* = HSB 210°, 9%, 49%; WCAG 2.0 level AA at 4.52:1 contrast ratio on `#fff` */
--color-filled-disabled:      var( --color-base-contrast );

we would feature

/* Foreground Colors */
--color-base:                 #222;
--color-base--hover:          #444;
--color-base--active:         #000;
--color-base--contrast:       #fff;
--color-base--emphasized:     var( --color-base--active );
--color-base--disabled:       #72777d; /* = HSB 210°, 9%, 49%; WCAG 2.0 level AA at 4.52:1 contrast ratio on `#fff` */
--color-filled--disabled:     var( --color-base--contrast );

Feedback welcome!

Event Timeline

Volker_E created this task.Oct 7 2016, 12:20 AM
Restricted Application added a project: UI-Standardization. · View Herald TranscriptOct 7 2016, 12:20 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Volker_E renamed this task from Variable names in WikimediaUI Base to Variable naming in WikimediaUI Base.Oct 7 2016, 12:21 AM

I think -invert makes more sense than -contrast. It gives the correct invert color for the purpose of the repo, just not mathematically, so I think its fine.

Not sure what you mean when you say, 'if the widget is filled disabled'. What does filled mean?

Don't know anything about the BEM style, but the double hyphens definitely make it more readable.

invert is probably the best word here as it means "put upside down or in the opposite position, order, or arrangement."

Personally I'd prefer we don't have variables for these names since they can be derived from other values and instead use mixins in the code itself where needed. The danger with a variable based approach is the variables can be come badly named after a while.

Consider a change where:

--color-base-active: #000;
--color-base-contrast: #fff;

becomes

--color-base-active: #000;
--color-base-contrast: #eee;

@Jdlrobson @Prtksxna Thanks for your feedback.
Going for -invert instead. Jon, I understand the sentiment about using Less functions/mixins, but from a designers perspective it is important to have those colors defined fixed somewhere. As it doesn't make a positive difference if you change the base-active color and don't think about the definition -invert, but rely on an automatic definition.
Then there's also the negative issue about involving a Less specific functionality, that needs to be translated if we might decide to change technology.

For those reasons, I'm eager to leaving those in for now. We could revisit -invert in future.

Volker_E triaged this task as Medium priority.Nov 11 2016, 1:58 AM
Volker_E added a project: Patch-For-Review.
Volker_E moved this task from Researching… to Review on the UI-Standardization-Kanban board.

@Jdlrobson @Prtksxna Thanks for your feedback.
Going for -invert instead.
[...]
We could revisit -invert in future.

It looks like the patch is featuring -inverted rather than -invert. I don't have a strong feeling about one vs the other, but I wanted to point it out in case one has.

@JGirault That's correct, I've decided that any modifier to a variable besides CSS own states :link, :visited, :active, :hover and :focus should be in passive voice.
Follow-up patch will care about wrong --highlighted color.

Further notice on naming scheme: CSS-property–descriptive-adjective--modifier-state, for example border-color-readonly--hover

Volker_E closed this task as Resolved.Dec 1 2016, 7:24 AM
Volker_E raised the priority of this task from Medium to High.
Volker_E removed a project: Patch-For-Review.
Volker_E moved this task from Review to Done on the UI-Standardization-Kanban board.