Change 768788 merged by jenkins-bot:

[design/codex@main] ToggleButton: add ToggleButton component

https://gerrit.wikimedia.org/r/768788

The initial implementation is merged, and can be viewed here: https://doc.wikimedia.org/codex/main/components/toggle-button.html

Design review questions:

• Does the "on" state use the right color (color-primary--active) ?
• What should the hover and active states look like when the button is "on"? Right now there are hover/active states for the "off" state, but not for the "on" state.

Change 769548 had a related patch set uploaded (by DannyS712; author: DannyS712):

[design/codex@main] Add button-input mixin to avoid style duplication

https://gerrit.wikimedia.org/r/769548

Change 769548 merged by jenkins-bot:

[design/codex@main] Add button styles mixin to avoid style duplication

https://gerrit.wikimedia.org/r/769548

Needs design review per above, code was merged

In T295179#7805947, @bmartinezcalvo wrote:

@DannyS712 I come back to you with the Figma component spec sheet for ToggleButton and the design review of the Codex demo.

After designing the Figma component spec sheet, we would need to make the following updates to the Codex demo:

1. Quiet button variant should be added
2. We need to add the Toggled-on Hover and Active states to indicate the user that come back to the default state is possible.
3. Text+Icon and Icon-Only variants should be added in the demo.

Here you can view the Figma spec sheet.

Here you can view the prototype of the ToggleButton (Normal and Quiet variants) to understand how the toggle-on hover and active work.

Also I have include it in the task description.

When i click on the figma link, I'm told "File not found" and

Either this file doesn’t exist or you don’t have permission to view it. Ask the file owner to verify the link and/or update permissions.

When i click on the figma link, I'm told "File not found" and

Either this file doesn’t exist or you don’t have permission to view it. Ask the file owner to verify the link and/or update permissions.

@DannyS712 I'm so sorry, I had to give you permission to view the Figma file. I've updated the link permissions so you can view the Figma link.

Since T303485 now requires this, we'd like to get these design updates implemented this week. I've confirmed with @DannyS712 that they'll be able to work on this task before the end of the week.

In T295179#7810201, @bmartinezcalvo wrote:

When i click on the figma link, I'm told "File not found" and

Either this file doesn’t exist or you don’t have permission to view it. Ask the file owner to verify the link and/or update permissions.

@DannyS712 I'm so sorry, I had to give you permission to view the Figma file. I've updated the link permissions so you can view the Figma link.

The figma doesn't appear to include the toggled on and disabled state, can you add it so I know what the quiet version should look like?

In T295179#7820170, @DannyS712 wrote:

The figma doesn't appear to include the toggled on and disabled state, can you add it so I know what the quiet version should look like?

Hey @DannyS712 you have in the spec sheet all Normal and Quiet states (view them here):

Also I've included the guides section for quiet buttons in the beginning of the spec sheet:

Tell me if you need something more.

In T295179#7820900, @bmartinezcalvo wrote:

In T295179#7820170, @DannyS712 wrote:

The figma doesn't appear to include the toggled on and disabled state, can you add it so I know what the quiet version should look like?

Hey @DannyS712 you have in the spec sheet all Normal and Quiet states (view them here):

Also I've included the guides section for quiet buttons in the beginning of the spec sheet:

Tell me if you need something more.

I mean I don't see the spec for the button when it is both toggled on *and* disabled

I mean I don't see the spec for the button when it is both toggled on *and* disabled

@DannyS712 do you mean the state Toggle-on Disabled? This design doesn't exist since we always use the same Disabled button for all button variants (Primary, Normal or Quiet). I mean, we don't have specific design for disabled outline buttons and disabled solid buttons, so we should use the same solid disabled button for both toggled disabled and toggled-on disabled.

In T295179#7822511, @bmartinezcalvo wrote:

I mean I don't see the spec for the button when it is both toggled on *and* disabled

@DannyS712 do you mean the state Toggle-on Disabled? This design doesn't exist since we always use the same Disabled button for all button variants (Primary, Normal or Quiet). I mean, we don't have specific design for disabled outline buttons and disabled solid buttons, so we should use the same solid disabled button for both toggled disabled and toggled-on disabled.

I don't really understand - what is the difference between "toggled disabled" and "toggled-on disabled"?

If you go to https://doc.wikimedia.org/codex/main/components/toggle-button.html, hit the button to toggle it on, and then disable the button, you can see that the disabled state still reflects that it was toggled on. I would expect that the quiet version also reflects whether it is toggled on or toggled off when it is disabled

If you go to https://doc.wikimedia.org/codex/main/components/toggle-button.html, hit the button to toggle it on, and then disable the button, you can see that the disabled state still reflects that it was toggled on. I would expect that the quiet version also reflects whether it is toggled on or toggled off when it is disabled

I hadn't realized when I did the Design Review of the demo but this design of the button when you disable it while toggled is not correct and should be fixed:

We don't want the disabled state for toggled-on is different for not toggled. Disabled buttons for both toggled and not toggled should be the same disabled button (that matches the rest of disabled on all our button):

Both normal and quiet buttons won't display a different disabled state for toggled and not toggled.

In T295179#7824152, @bmartinezcalvo wrote:

If you go to https://doc.wikimedia.org/codex/main/components/toggle-button.html, hit the button to toggle it on, and then disable the button, you can see that the disabled state still reflects that it was toggled on. I would expect that the quiet version also reflects whether it is toggled on or toggled off when it is disabled

I hadn't realized when I did the Design Review of the demo but this design of the button when you disable it while toggled is not correct and should be fixed:

We don't want the disabled state for toggled-on is different for not toggled. Disabled buttons for both toggled and not toggled should be the same disabled button (that matches the rest of disabled on all our button):

Both normal and quiet buttons won't display a different disabled state for toggled and not toggled.

Respectfully, I disagree.
I originally wrote the ToggleButton component for MediaWiki-extensions-GlobalWatchlist (T259403 and 2a394caec0ca7bb31d1fa0782517cb85f6d28d97). The toggled on and disabled styles were based on OOUI, see here. This was later moved to WVUI (T287135 and 0b08d121d0f8ad3e3df415137206ea1e75da1e4f) and then added to codex in this task.
In global watchlist, there is a boolean toggle option that is sometimes disabled from interactions, but the existing state is still used, so it is shown. Thats why I made sure that the Vue version had a display difference the same way that the OOUI toggle does. If it is removed from the codex version, I'll have to recreate it in global watchlist to add the support back, which is undesirable since we are creating a shared library to reduce such duplication.

Perhaps we can add a boolean prop for whether the "toggled-on disabled" state should reflect being toggled on, or just match the "toggled-off disabled" state? We can even have it default to matching. I just think there should be some way to have the state visible while disabled.

Under the instructions for contributing to codex, at https://doc.wikimedia.org/codex/main/contributing/contributing-code.html#component-addition-process, it says

Gather relevant design artifacts. Before a component can be added to Codex, it should have a complete entry in the Wikimedia Design Style Guide. There may be exceptions to this, in which case the component should match OOUI styles.

The design style guide includes no discussion that I can see of the disabled and toggled-on state, so it is entirely appropriate to match OOUI and support a visual difference for toggled on disabled and toggled off disabled

In T295179#7825601, @DannyS712 wrote:

Under the instructions for contributing to codex, at https://doc.wikimedia.org/codex/main/contributing/contributing-code.html#component-addition-process, it says

Gather relevant design artifacts. Before a component can be added to Codex, it should have a complete entry in the Wikimedia Design Style Guide. There may be exceptions to this, in which case the component should match OOUI styles.

The design style guide includes no discussion that I can see of the disabled and toggled-on state, so it is entirely appropriate to match OOUI and support a visual difference for toggled on disabled and toggled off disabled

That documentation is out of date, and we should update it to better reflect the process we are now following, which involves component spec sheets in Figma. The spec sheets in Figma are supposed to be the source of truth now.

In global watchlist, there is a boolean toggle option that is sometimes disabled from interactions, but the existing state is still used, so it is shown. Thats why I made sure that the Vue version had a display difference the same way that the OOUI toggle does.

Could you provide more information about this use case, with screenshots? I think that would help everyone better understand why this is needed.

In order to move forward for now, I suggest we:

• Continue implementing the quiet state, so that we can merge that patch and the Abstract Wikipedia team can start using it. There seems to be no disagreement on the design for the quiet enabled case, as far as I can tell.
  • For the quiet disabled state, this would use the same styling as a quiet disabled button, for now, but we would re-evaluate that
• File a new task to discuss button disabled states. @bmartinezcalvo has an idea for a proposal where a disabled normal button would look different from a disabled primary button (right now they look the same, and a disabled normal button looks kind of primary-like, with a background color and inverted text), and then the disabled state for a toggled-on button could follow that of a primary button, while toggled-off would follow normal. Because this affects both ToggleButtons and regular Buttons, I think it would be best to discuss this in a separate task.

In T295179#7830114, @Catrope wrote:

That documentation is out of date, and we should update it to better reflect the process we are now following, which involves component spec sheets in Figma. The spec sheets in Figma are supposed to be the source of truth now.

Hey @DannyS712 as Roan comments, that documentation in Codex is not reflecting our current workflow since in our current workflow the Figma spec sheets are always the source of truth before the component is developed in Codex.

About the implementation, as Roan comments it's better if we can implement the Quiet variant to be make it available to use in Abstract Wikipedia project.

To move forward with this ask I've update the ToggleButton component spec sheet with disabled states for both toggled-off and toggled-on.

At the moment both disabled states have the same visual design because currently our primary and normal buttons use the same disabled style. As toggled-off is visually a normal button and toggled-on is a primary-active one, the logical solution now is using the same disabled button that we use in the Button component.

About having a different disabled state for toggled-off and toggled-on, first of all, we need to understand better the use cases where you need to use these toggled-off/on disabled buttons. Once we have checked these use cases we will be able to evaluate if it's necessary to update this toggled-off disabled. As toggled-off is using a normal button, I'm going to create a task to evaluate if we want our normal disabled buttons to look different from the primary disabled buttons.

While we evaluate it and study the possible use cases of the toggled-off/on disabled, you can continue with the development of the quiet buttons so the task is not blocked.

Okay, for the quiet version I'll have disabled be the same for toggled-on and toggled-off

In T295179#7833691, @DannyS712 wrote:

Okay, for the quiet version I'll have disabled be the same for toggled-on and toggled-off

@DannyS712 right, in the case of quiet buttons we will have these buttons without background and gray text for both toggled-on and toggled-off.

Change 780635 had a related patch set uploaded (by DannyS712; author: DannyS712):

[design/codex@main] ToggleButton: add quiet mode

https://gerrit.wikimedia.org/r/780635

Demo of this patch: https://780635--wikimedia-codex.netlify.app/components/toggle-button.html

@bmartinezcalvo For quiet ToggleButtons, the hover state looks like same for toggled-off and toggled-on. Is that intended? It feels confusing to me, because when you click the button to toggle it (from off to on, or from on to off), you are of course hovering it, so you don't see its appearance change until you move the mouse away.

@DannyS712, thank you for your work on this component! @bmartinezcalvo @Catrope I'm going to move this to our current sprint board to track progress. It looks like we are all set with Figma now so I checked "Publish Figma library with all message updates" as complete. I'm also going to create a task for to update the Codex documentation with new copy. We might want to just link out to the MediaWiki page for all contribution instructions for consistency.

Change 780635 merged by jenkins-bot:

[design/codex@main] ToggleButton: add quiet type

https://gerrit.wikimedia.org/r/780635

In T295179#7857005, @Catrope wrote:

Demo of this patch: https://780635--wikimedia-codex.netlify.app/components/toggle-button.html

@bmartinezcalvo For quiet ToggleButtons, the hover state looks like same for toggled-off and toggled-on. Is that intended? It feels confusing to me, because when you click the button to toggle it (from off to on, or from on to off), you are of course hovering it, so you don't see its appearance change until you move the mouse away.

@Catrope it was intended. Hover is using Base90 for both toggled-off and toggled-on because we don't have an intermediate color between Base80 and Base90 in our gray scale, so we need to use Base90 #EAECF0 for both.

In T295179#7858109, @STHart wrote:

It looks like we are all set with Figma now so I checked "Publish Figma library with all message updates" as complete.

@STHart this "Publish Figma library" step should be done once the task is in the Pending release" column, since the component demo could have corrections to do (in fact, I'm going to add some corrections found during the Design Review). Publish the component on Figma means that this component will be merged with our main library and all designers will be able to use it, so we need to make sure that the component has no further fixes to do.

@DannyS712 I've done the Design Review of the component demo and I found some corrections:

1. We need to add toggled-on-hover and toggled-on-active too for normal button to indicate the user the interaction when the user wants to turn off the button again.

2. Disabled toggled-on (normal button) still has wrong styles. It should be exactly the same as disabled toggled-off. Quiet disabled states are ok.

3. The following button variants are missing in the configurable demo:
4. With icon (Icon + Text)
5. Only Icon

In T295179#7863835, @bmartinezcalvo wrote:

@DannyS712 I've done the Design Review of the component demo and I found some corrections:

1. We need to add toggled-on-hover and toggled-on-active too for normal button to indicate the user the interaction when the user wants to turn off the button again.

2. Disabled toggled-on (normal button) still has wrong styles. It should be exactly the same as disabled toggled-off. Quiet disabled states are ok.

As explained above, I strongly disagree, there should be a way to visually distinguish between a disabled button that is toggled-on and one that is toggled-off, which I use in the GlobalWatchlist extension. Eg there is an option to group changes by page, which is a toggle button. While the data is being reloaded, the option cannot be changed, so it is disabled, but the state is still applied, so the button shows if its enabled or not

3. The following button variants are missing in the configurable demo:

We don't yet have support for configurable icons in demos, I don't think this should be a blocker for this task of adding the component itself

• With icon (Icon + Text)
• Only Icon

NOTE: I'm going to add these corrections in the task description too so we can check all of them. Also, I'm going to move the task to the Ready for Development column to fix this corrections list.

In T295179#7865483, @DannyS712 wrote:

As explained above, I strongly disagree, there should be a way to visually distinguish between a disabled button that is toggled-on and one that is toggled-off, which I use in the GlobalWatchlist extension. Eg there is an option to group changes by page, which is a toggle button. While the data is being reloaded, the option cannot be changed, so it is disabled, but the state is still applied, so the button shows if its enabled or not

@DannyS712 about the disabled buttons I wrote the following in a previous comment:

In T295179#7831072, @bmartinezcalvo wrote:

At the moment both disabled states have the same visual design because currently our primary and normal buttons use the same disabled style. As toggled-off is visually a normal button and toggled-on is a primary-active one, the logical solution now is using the same disabled button that we use in the Button component.

About having a different disabled state for toggled-off and toggled-on, first of all, we need to understand better the use cases where you need to use these toggled-off/on disabled buttons. Once we have checked these use cases we will be able to evaluate if it's necessary to update this toggled-off disabled. As toggled-off is using a normal button, I'm going to create a task to evaluate if we want our normal disabled buttons to look different from the primary disabled buttons.

Currently both primary (solid) and normal (outline) buttons in our system use the same disabled style, so ToggleButtons should use the same styles since they are created with the same styles as buttons. I created this task T305437 to evaluate if we want to update or not our disabled normal buttons, but until we decide to update them, the toggled-off (outline) should use the same disabled as the normal (outline) button.

In T295179#7865483, @DannyS712 wrote:

We don't yet have support for configurable icons in demos, I don't think this should be a blocker for this task of adding the component itself

About icons in Codex demos, currently we have configurable icons in the TextInput component, so I think we should be able to implement it for the icons too, right @Catrope ?

Change 784770 had a related patch set uploaded (by Catrope; author: Catrope):

[design/codex@main] ToggleButton: Update quiet styles and make focusable

https://gerrit.wikimedia.org/r/784770

Change 784771 had a related patch set uploaded (by Catrope; author: Catrope):

[design/codex@main] ToggleButton: Add icon-only demo

https://gerrit.wikimedia.org/r/784771

In T295179#7869702, @gerritbot wrote:

Change 784770 had a related patch set uploaded (by Catrope; author: Catrope):

[design/codex@main] ToggleButton: Update quiet styles and make focusable

https://gerrit.wikimedia.org/r/784770

This patch (demo here) implements the following:

• Adds toggled-on-hover and toggled-on-active styles for framed (non-quiet) buttons (#1 from @bmartinezcalvo's list)
• Makes the disabled state for quiet buttons the same for toggled on as toggled off (#2 on the list)
  • I disagree with this, but we can discuss further on T305437)
• Makes toggle buttons focused on click, like other buttons
  • Is this desired? I kind of assumed that it would be, since it's more consistent with normal buttons and checkboxes, but there must have been a reason why the initial implementation made ToggleButtons not focus on click (using @mousedown.prevent)
  • One downside is that it makes the issue of the quiet button not seeming responsive worse, see below
• Implements the hover/focus/active states for quiet buttons, as specified in Figma
  • However, the hover and focus states for quiet buttons are all the same for toggled-on vs toggled-off (even the background color on focus!), which I don't think is a good idea. It makes the button appear not to respond to clicks, and you have to move the mouse off of them before you can see that the toggled state has changed (and, if we decide to make toggle buttons focused on click as this patch does, you have to also click somewhere else to move the focus away).

In T295179#7867044, @bmartinezcalvo wrote:

About icons in Codex demos, currently we have configurable icons in the TextInput component, so I think we should be able to implement it for the icons too, right @Catrope ?

Yes, we could do that, but this conflicts with the generated code sample feature. We would need to either disable that feature for buttons (which is what TextInput did), or make improvements to it so that this is supported. This is tracked at T304042.

...however, 784771 (demo) does add an icon-only version of the "stateful" demo, so that we have both icon+text and icon-only covered (just as static demos for now, not configurable ones)

In T295179#7869879, @Catrope wrote:

In T295179#7867044, @bmartinezcalvo wrote:

About icons in Codex demos, currently we have configurable icons in the TextInput component, so I think we should be able to implement it for the icons too, right @Catrope ?

Yes, we could do that, but this conflicts with the generated code sample feature. We would need to either disable that feature for buttons (which is what TextInput did), or make improvements to it so that this is supported. This is tracked at T304042.

In T295179#7869881, @Catrope wrote:

...however, 784771 (demo) does add an icon-only version of the "stateful" demo, so that we have both icon+text and icon-only covered (just as static demos for now, not configurable ones)

@DannyS712 @Catrope then if we can't include at the moment configurable icons in our demos, then show the StartIcon and IconOnly variants in the stateful demos below the configurable demo.

In T295179#7870766, @bmartinezcalvo wrote:

@DannyS712 @Catrope then if we can't include at the moment configurable icons in our demos, then show the StartIcon and IconOnly variants in the stateful demos below the configurable demo.

Does this look good?

In T295179#7872525, @Catrope wrote:

In T295179#7870766, @bmartinezcalvo wrote:

@DannyS712 @Catrope then if we can't include at the moment configurable icons in our demos, then show the StartIcon and IconOnly variants in the stateful demos below the configurable demo.

Does this look good?

@Catrope yes, it might work for now to show the Icon+Text and Icon-Only variants.

One correction about the Icon-Only variant. This variant should be 32x32px for both Button and ToggleButtons components and currently it's wider than tall in Codex.

We should fix it. Should we add a new correction item in this checklist task or should we create another task to fix both Button and ToggleButton components?

Change 784770 merged by jenkins-bot:

[design/codex@main] ToggleButton: Update quiet styles and make focusable

https://gerrit.wikimedia.org/r/784770

In T295179#7876470, @bmartinezcalvo wrote:

One correction about the Icon-Only variant. This variant should be 32x32px for both Button and ToggleButtons components and currently it's wider than tall in Codex.

We should fix it. Should we add a new correction item in this checklist task or should we create another task to fix both Button and ToggleButton components?

I would prefer creating a new task for this, especially since it also affects Button.

Actually it looks like making icon-only buttons square is already part of T301996.

Change 784771 merged by jenkins-bot:

[design/codex@main] ToggleButton: Add icon-only demo

https://gerrit.wikimedia.org/r/784771

Design Sign-Off done. All items from the checklist have been fixed. About the one to include the icons in the configurable demo, we will show buttons with icons in the stateful demo until the icons can be configured in the configurable demo.

Moving Functional Testing of this component out of this current sprint and into the general Team Functional Testing column, in order to focus on current sprint goals, which primarily include preparing Typeahead Search and several other smaller tasks.

A related reason for doing this is to ensure we have time to triage any tickets or followup work that might come from the functional review of this fix.

Will pick up when prioritized for release.