Page MenuHomePhabricator

Consolidate `oo-ui-buttonElement-active` and `-pressed` CSS Classes in OOUI
Open, LowPublic

Description

Currently OOUI has implemented three functions setActive() and ( onKeyDown() || onMouseDown() ) that add classes .oo-ui-buttonElement-active and .oo-ui-buttonElement-pressed respectively.
I don't see this a useful separation, we should follow CSS standard way (:active) of naming those classes for mousedown/pressed/active state and consolidate them into .oo-ui-buttonElement-active.

Event Timeline

Volker_E created this task.Oct 5 2015, 10:13 PM
Volker_E raised the priority of this task from to Needs Triage.
Volker_E updated the task description. (Show Details)
Volker_E added projects: UI-Standardization, OOUI.
Volker_E added subscribers: violetto, Esanders, matmarex and 2 others.
Volker_E moved this task from Backlog to Next-up on the OOUI board.Oct 5 2015, 10:17 PM

Adding comment by @matmarex in T113116#1688533:

In T113116#1662160, @Volker_E wrote:
Right now, there are several kinds of semantically pretty similar classes living side-by-side:
-active &
-pressed (like in .oo-ui-buttonElement-active/.oo-ui-buttonElement-pressed (In CSS active stands for pressed?),
These are indeed pretty weird and pretty mixed up. Perhaps we could clean them up with some effort.
-enabled (like in .oo-ui-widget-enabled)
This is for enabled/disabled state, like <button disabled>Foo</button> in HTML.
-on (like in .oo-ui-toggleWidget-on)
This is for "checkedness" state, like <input type=checkbox checked> in HTML.

The buttonOptionWidget f.e is currently getting both -pressed and -active classes to differentiate the currently selected/on state as -active and the mousedown/pressed/:active state as -pressed.
Wouldn't it be more coherent to change that -active to an -on like with the ToggleSwitchWidget?

@Prtksxna I've seen in history.md there's been a "counter-movement" renaming @active to @pressed. Please explain the reasons behind back then.

@Prtksxna I've seen in history.md there's been a "counter-movement" renaming @active to @pressed. Please explain the reasons behind back then.

There were inconsistencies in how we were using those two words. The inconsistencies might still exists, but our intention was to use @pressed for the :active state, and @active when the element was active in the OOjs UI sense, like a toggle button that is pressed, or a menu option that is selected.

Jdforrester-WMF triaged this task as Low priority.Nov 17 2015, 1:25 AM
Jdforrester-WMF set Security to None.
Volker_E added a comment.EditedApr 20 2016, 1:02 AM

-active &
-pressed (like in .oo-ui-buttonElement-active/.oo-ui-buttonElement-pressed (In CSS active stands for pressed?),

These are indeed pretty weird and pretty mixed up. Perhaps we could clean them up with some effort.

-enabled (like in .oo-ui-widget-enabled)

This is for enabled/disabled state, like <button disabled>Foo</button> in HTML.

-on (like in .oo-ui-toggleWidget-on)

This is for "checkedness" state, like <input type=checkbox checked> in HTML.
The buttonOptionWidget f.e is currently getting both -pressed and -active classes to differentiate the currently selected/on state as -active and the mousedown/pressed/:active state as -pressed.

The buttonOptionWidget receives -selected as well.
Then there's also -depressed.
Then there's also -highlighted which refers to :hover in selectWidgets/optionWidgets (dropdown menus) if I'm correct? I'm getting dizzy.

Is there a way forward to simplify those @matmarex @Esanders @Prtksxna?

What is your take on the following:

Current stateProposed stateExplanation
-enabled/-disabledremainsGeneral state of 'enablement' of widgets
-active/-selected/-onmerge into -onIf a widget is on/selected independent from its state above. Going for the smallest one, the light switch/power button state of user interfaces
-highlightedremove?What are the special use cases/x-browser issues where CSS' :hover falls short?
-pressedreplace with -active?There's onkeydown/onmousedown in JS (leaving behind onkeypress, there's :active in CSS.
-depressed?Don't know about the origin of this one
Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptApr 20 2016, 1:02 AM

I think it'll prevent some dizziness if we use the full class names, since they indicate where they come from. The class explosion is largely caused by the way we compose our Widgets from the Element mixins. So:

CSS class name Source JS class
oo-ui-widget-enabledWidgetExplained above. There is also oo-ui-widget-disabled (the opposite), because that's easier to write in CSS than :not() selectors or overriding less specific styles.
oo-ui-buttonElement-activeButtonElementThis is nebulous. (But see below the table.)
oo-ui-buttonElement-pressedButtonElementThis is set if the button is currently being pressed. The user clicked it and is still holding the mouse button, or they had it focused and pressed Enter on the keyboard and is still holding the key.
oo-ui-toggleWidget-onToggleWidgetThis is set if the toggle is in the "on" state. It is valid for all ToggleWidgets. We can't remove it to make the styling of ToggleButtonWidget simpler, because that would break ToggleSwitchWidget. There is also oo-ui-toggleWidget-off (the opposite), because that's easier to write in CSS than :not() selectors or overriding less specific styles.
oo-ui-optionWidget-highlightedOptionWidgetThis is set if the option is highlighted, either by mouse (same as CSS :hover) or by keyboard (use keyboard arrow keys; no CSS equivalent). ButtonOptionWidget has no styles for this, but e.g. MenuOptionWidget does.
oo-ui-optionWidget-selectedOptionWidgetThis is set if the option is selected. It is valid for all OptionWidgets. We can't remove it to make the styling of ButtonOptionWidget simpler, because that would break MenuOptionWidget.
oo-ui-optionWidget-pressedOptionWidgetThis is set if the option is currently being pressed. The user clicked it and is still holding the mouse button (no keyboard handling here).
oo-ui-selectWidget-pressedSelectWidgetThis is set if any option inside the select widget is currently being pressed. The user clicked it and is still holding the mouse button (no keyboard handling here). There is also oo-ui-selectWidget-depressed (the opposite), because that's easier to write in CSS than :not() selectors or overriding less specific styles.

Se what is the deal with oo-ui-buttonElement-active? Apparently it's just a workaround to avoid specifying the same styles twice for .oo-ui-buttonOptionWidget.oo-ui-optionWidget-selected and .oo-ui-toggleButtonWidget.oo-ui-toggleWidget-on. But I just remembered another use for it that I thought of some time ago, and implemented it: https://gerrit.wikimedia.org/r/284529 (marking links in navigation menus shaped like button as active).

The only classes we could merge are those coming from the same source, and it doesn't seem like any of them have overlapping functionality. So I don't think we can merge oo-ui-buttonElement-active and oo-ui-buttonElement-pressed after all, nor anything else.

But I'd be open to renaming some of them to avoid ambiguity. Some of the current CSS class names don't reflect the subtle differences very well.

Danny_B moved this task from Unsorted to OOUI on the UI-Standardization board.May 20 2016, 9:01 PM

After a few months and getting to know the source code more closely, I'd recommend to get

  • .oo-ui-buttonElement-active replaced by .oo-ui-buttonElement-on and
  • .oo-ui-buttonElement-pressed removed and replaced just by :active modifiers.

I guess, that -pressed has been invented to address a IE <= 9 issue where without it CSS :active state wasn't correctly represented on input elements. As we also take care about :active:focus and :focus styling I think that is a insignificant problem and removing -pressed would simplify understandability of the code, by just relying on CSS built-in state modifiers, adding just the -on state which has no simple CSS representation.

Volker_E renamed this task from Consolidate `oo-ui-buttonElement-active` and `-pressed` CSS Classes in OOjs UI to Consolidate `oo-ui-buttonElement-active` and `-pressed` CSS Classes in OOUI.Jan 6 2018, 7:50 AM
Volker_E updated the task description. (Show Details)
Volker_E removed a subscriber: violetto.