In T311756#8126329, @Catrope wrote:

Another outcome of our code review session (and comments on Gerrit) is that @AnneT suggested using the name ButtonGroup for this component instead of ActionButtonGroup, for symmetry. A group of Buttons would be a ButtonGroup, and a group of ToggleButtons would be a ToggleButtonGroup. There is no ActionButton, and "Action" doesn't appear in the name of anything else either. @Volker_E, @egardner and myself agreed, but we didn't want to unilaterally rename this component without asking @bmartinezcalvo and @Sarai-WMDE

@Catrope it makes sense. At the beginning we decided to name it "Action ButtonGroup" to reflect the type of action the button had (actionable or toggled). But as you comment the ButtonGroup is a group of buttons and the Toggle ButtonGroup a group of ToggleButtons so it makes sense to me. I'm going to update the name of the component in the Figma spec sheet.

Change 815364 merged by jenkins-bot:

[design/codex@main] ButtonGroup: Initial implementation

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

@Catrope I've reviewed the ButtonGroup and we should update a couple of things:

1. Maximum example section is missing
2. Avoid to use go back and go next buttons (and icons) in the examples since the button group buttons will be used more for actions (such as delete, edit, comment...)

3. Stacked buttons were not updated, what can we do? We should solve it somehow since the current solution is not good.

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

[design/codex@main] ButtonGroup: Add overflowing demo, fix styling

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

In T311756#8136906, @bmartinezcalvo wrote:

@Catrope I've reviewed the ButtonGroup and we should update a couple of things:

1. Maximum example section is missing

Added in the attached patch.

2. Avoid to use go back and go next buttons (and icons) in the examples since the button group buttons will be used more for actions (such as delete, edit, comment...)

I've changed the "go back" button to a "download" button.

3. Stacked buttons were not updated, what can we do? We should solve it somehow since the current solution is not good.

I was able to fix this better than I thought. It's not entirely optimal, but it's better than what we had before. The misalignment between the left edges is fixed, and the double bottom/top borders are fixed too. The only thing that's not right is the rounded corners, and I don't think fixing those will be possible:

One issue that I'm still trying to figure out if it's possible to fix is that when the buttons overflow to the next line between two disabled buttons, the first one on the new line appears to be off by 1 pixel (this is because the white border between two disabled buttons is implemented as the left border of the second disabled button).

The same thing happens when there are two toggled-on toggle buttons next to each other and the line breaks in between:

For context, I should add that I looked into having the buttons not wrap and all stay on the same line, with overly long buttons being truncated with ellipses. But due to the way flexbox works, all the buttons get shrunk by equal amounts, so that's not very helpful:

If I set flex-basis: 0; flex-grow: 1, that makes all of the buttons equally wide, which looks much better:

But the downside is that that makes the buttons as wide as they can be, even if that width isn't needed:

The design spec does include a (non-MVP) "full width" mode that looks exactly line the above. So I propose that we use overflow to the next line in non-full-width mode, and ellipses in full-width mode. (Doing anything else would be difficult CSS-wise.)

@Catrope design sign off done, all works well for me now. The only thing that doesn't convince me is the rounded corners in the stacked buttons but in this last patch they are much better so I think we can move forward with this version of the stacked buttons.

Regarding the maximum example when the text is too long, if the buttons are stacked in rows when there is no enough space for them, I think we can delete this example with ellipsis in the text from the Figma spec sheet and show a maximum example with long buttons instead (since in the first maximum example the stacked buttons are explained). If you agree, I'm going to update this example to avoid confusions.

{F35413519}

In T311756#8140941, @bmartinezcalvo wrote:

@Catrope design sign off done, all works well for me now. The only thing that doesn't convince me is the rounded corners in the stacked buttons but in this last patch they are much better so I think we can move forward with this version of the stacked buttons.

Yeah, I'm sorry about that. I agree that the rounded corners should look the way you designed them, but I can't find a way to make that happen. CSS doesn't let us apply special styles to the first/last element on a line, only to the first/last element in the overall group. It might be possible to use JavaScript to check the coordinates of each button every time the window is resized and figure out which buttons should have rounded corners that way, but that seems like overkill for a 1px border-radius style.

Regarding the maximum example when the text is too long, if the buttons are stacked in rows when there is no enough space for them, I think we can delete this example with ellipsis in the text from the Figma spec sheet and show a maximum example with long buttons instead (since in the first maximum example the stacked buttons are explained). If you agree, I'm going to update this example to avoid confusions.

{F35413519}

That's fine with me. However, we might want to keep that example in our back pocket for when we implement "full width" mode later, because I think we'll want the maximum example in with long buttons in full width mode to look exactly like that.

In T311756#8141121, @Catrope wrote:

Yeah, I'm sorry about that. I agree that the rounded corners should look the way you designed them, but I can't find a way to make that happen. CSS doesn't let us apply special styles to the first/last element on a line, only to the first/last element in the overall group. It might be possible to use JavaScript to check the coordinates of each button every time the window is resized and figure out which buttons should have rounded corners that way, but that seems like overkill for a 1px border-radius style.

@Catrope what I made in design to solve this was to add all buttons in the button group with 0px border radius and then add a 2px border radius just in the button group box, so that all the buttons contained in that box are always rounded on both left and right sides. Could you add the same solution in code? Not all buttons would be rounded but at least all corners in the button group would be rounded.

{F35419234}

In T311756#8141121, @Catrope wrote:

That's fine with me. However, we might want to keep that example in our back pocket for when we implement "full width" mode later, because I think we'll want the maximum example in with long buttons in full width mode to look exactly like that.

@Catrope ok, added text with ellipsis in the maximum examples for full-width button groups and added a note explaining that it won't be implemented in the MVP.

In T311756#8142532, @bmartinezcalvo wrote:

@Catrope what I made in design to solve this was to add all buttons in the button group with 0px border radius and then add a 2px border radius just in the button group box, so that all the buttons contained in that box are always rounded on both left and right sides. Could you add the same solution in code? Not all buttons would be rounded but at least all corners in the button group would be rounded.

{F35419234}

I can't see that image, can you make it public?

When I try this, I get double borders like this, and the top left corner isn't rounded. I think the double border along the bottom is because of the margin-top: -1px on the buttons (which pulls them up so that their borders overlap the borders of the buttons above them) and I might be able to fix that, but along the top border it's also not doing what we want.

Change 821321 merged by jenkins-bot:

[design/codex@main] ButtonGroup: Add overflowing demo, fix styling

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

In T311756#8142922, @Catrope wrote:

I can't see that image, can you make it public?

When I try this, I get double borders like this, and the top left corner isn't rounded. I think the double border along the bottom is because of the margin-top: -1px on the buttons (which pulls them up so that their borders overlap the borders of the buttons above them) and I might be able to fix that, but along the top border it's also not doing what we want.

@Catrope I've made the file public so you can watch the video I attached.

Regarding the solution for stacked buttons, I designed them with the following:

• All buttons inside the button group with 0px border radius
• Button group with 2px border radius in all sides, but without any border color applied

You can inspect this design with the stacked buttons so you can understand better how it's built. Also, we can schedule a quick call if needed to explain you better how I built the button group with stacked buttons.

Apart from this, I think the current solution in the demo is a good solution. It would be great that we could add the right border radius but I know it's difficult and reviewing other DS button groups this seems to be a recurring problem when buttons are stacked in multiple rows, and there doesn't seem to be an easy solution to fix it. So, as this is an MVP task I would go with what we have now and I would fix it in the future if we find a better solution.

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

[design/codex@main] ButtonGroup: Apply rounded corners to groups, not buttons

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

In T311756#8144901, @bmartinezcalvo wrote:

@Catrope I've made the file public so you can watch the video I attached.

Thanks!

Regarding the solution for stacked buttons, I designed them with the following:

• All buttons inside the button group with 0px border radius
• Button group with 2px border radius in all sides, but without any border color applied

You can inspect this design with the stacked buttons so you can understand better how it's built. Also, we can schedule a quick call if needed to explain you better how I built the button group with stacked buttons.

I tried this in the browser and at first I thought it didn't work, and that it wasn't possible for a parent element's border-radius to affect the children's borders, especially if the parent had no border itself. But it turns out I just needed to set overflow: hidden on the parent, and that makes it work!

This reliably rounds the top left and bottom left corners, but not the top right and bottom right corners, because the button group's box takes up the potential width of the group, rather than the width of its contents (and even then, the widths of the rows are uneven anyway). But I think this is the best we can do.

Thanks for coming up with this solution and taking the time to explain it! I've implemented it in the attached patch.

@Catrope I just noticed this when looking at the all-disabled example (in Safari). Is this expected?

In T311756#8147068, @egardner wrote:

@Catrope I just noticed this when looking at the all-disabled example (in Safari). Is this expected?

Unfortunately yes. This is the same issue as when the line breaks between two adjacent toggled-on ToggleButtons, which is discussed on T310768. What happens here is that we have a CSS rule along the lines of .cdx-button-group .cdx-button:disabled + .cdx-button:disabled { border-left-color: white; } to make those white lines between the buttons. When the line wraps between two disabled buttons, that white border shows up as a misalignment instead of a line between the buttons. Ideally we would instead set the border-right-color on the first disabled button, but CSS doesn't provide a way to target the first of a pair of adjacent elements, only the second.

I figured as much. I still think this patch is an improvement (and the trick about using overflow: hidden to clip border radius is a good one).

Change 822446 merged by jenkins-bot:

[design/codex@main] ButtonGroup: Apply rounded corners to groups, not buttons

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

In T311756#8147026, @Catrope wrote:

This reliably rounds the top left and bottom left corners, but not the top right and bottom right corners, because the button group's box takes up the potential width of the group, rather than the width of its contents (and even then, the widths of the rows are uneven anyway). But I think this is the best we can do.

@Catrope it looks so much better than the first iteration and I also think we don't have any other way to round the right borders (unless we apply the full-width prop to the button group where the top right buttons will be aligned and there won't be so many unrounded corners, but this is not part of our MVP). So I think it's ok with this last iteration.

In T311756#8147080, @Catrope wrote:

In T311756#8147068, @egardner wrote:

@Catrope I just noticed this when looking at the all-disabled example (in Safari). Is this expected?

Unfortunately yes. This is the same issue as when the line breaks between two adjacent toggled-on ToggleButtons, which is discussed on T310768. What happens here is that we have a CSS rule along the lines of .cdx-button-group .cdx-button:disabled + .cdx-button:disabled { border-left-color: white; } to make those white lines between the buttons. When the line wraps between two disabled buttons, that white border shows up as a misalignment instead of a line between the buttons. Ideally we would instead set the border-right-color on the first disabled button, but CSS doesn't provide a way to target the first of a pair of adjacent elements, only the second.

@Catrope would it be possible to implement border-left-color: white in the toggled and disabled buttons instead border-left-color: white? In this way, the border would be visible at the last button in the row instead of in the fist one, so the white line wouldn't create that false misalignment effect.

In T311756#8148424, @bmartinezcalvo wrote:

@Catrope it looks so much better than the first iteration and I also think we don't have any other way to round the right borders (unless we apply the full-width prop to the button group where the top right buttons will be aligned and there won't be so many unrounded corners, but this is not part of our MVP). So I think it's ok with this last iteration.

Thanks! And yes you're right, in full-width mode all the corners will definitely be rounded.

@Catrope would it be possible to implement border-left-color: white in the toggled and disabled buttons instead border-left-color: white? In this way, the border would be visible at the last button in the row instead of in the fist one, so the white line wouldn't create that false misalignment effect.

I assume you mean setting border-right-color: white. As I said in T311756#8147080, we can't target the first button in a pair of adjacent disabled buttons, only the second one. But we could make the right border white on all disabled/toggled buttons, then try to clean up the consequences. I tried that, but I couldn't make it work. The main issue is that the right border of each button is usually not visible, because it's below the left border of the next button. To make it visible, I tried moving the next button over by a pixel so there's no overlap, but that just introduces the same misalignment problem for a different reason. Another issue is that toggled buttons are in a layer above the other buttons, so the extra white border would also be visible when a toggled button is next to a non-toggled button. (Disabled buttons don't have this problem because they're in a layer below the other buttons.)

Change 823725 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[mediawiki/core@master] Update Codex from v0.1.0-alpha.9 to v0.1.0-alpha.10

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

Change 823725 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.1.0-alpha.9 to v0.1.0-alpha.10

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

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

[design/codex@main] ButtonGroup: Use box-shadow instead of border between disabled buttons

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

I figured it out: if we use a box-shadow instead of a border, stacked disabled buttons work perfectly without issues, and it even works for vertically adjacent disabled buttons.

@Catrope I move the task to In Development since we need to solve the token treatment for the shadow used to separate the disabled buttons. Also we should fix the white line in the stacked buttons as I commented above.

Not sure if this should be the right column to move the task so feel free to move it again if needed.

Change 824559 merged by jenkins-bot:

[design/codex@main] ButtonGroup: Use box-shadow instead of border between disabled buttons

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

With the box-shadow patch being merged, this is now ready for design sign-off again.

As part of figuring out the tokenization for the shadow, we also improved what it looks like a little bit: the shadow no longer stops 1px short of the edge of the button as you showed in your previous screenshot, it now looks like this:

Hi @bmartinezcalvo, can you please confirm the designs below for a focused state?

1. In the design, the grey border of the adjacent button stands out but in the implementation, there is no such border.

2. When in the focused state (by long pressing the mouse key on a button), the radius of the button becomes really rounded as opposed to being straight in the design.

In T311756#8196718, @EUdoh-WMF wrote:

Hi @bmartinezcalvo, can you please confirm the designs below for a focused state?

1. In the design, the grey border of the adjacent button stands out but in the implementation, there is no such border.

@EUdoh-WMF the reason why the focus in Figma is with this gray line is why the buttons in the button group are -1px spacing (I used this implementation in Figma so the gray outline of the buttons are 1px). The buttons are stacked on top of each other and I can't control in Figma which button I add first. So the right implementation here is the one in the Codex demo.

2. When in the focused state (by long pressing the mouse key on a button), the radius of the button becomes really rounded as opposed to being straight in the design.

It's true, I hadn't realized that this happened with the focus buttons. @Catrope why are they rounded when pressing the focus button?

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

[mediawiki/core@master] Update Codex from v0.1.0-alpha.10 to v0.1.0

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

Change 828083 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.1.0-alpha.10 to v0.1.1

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

In T311756#8196718, @EUdoh-WMF wrote:

2. When in the focused state (by long pressing the mouse key on a button), the radius of the button becomes really rounded as opposed to being straight in the design.

This doesn't happen for me in Chrome on Ubuntu or in Firefox on Ubuntu. Which browser did you see this in? Maybe it's specific to Mac OS or specific to Safari.

What I do see in Chrome on Ubuntu is that if I l press the Space key when a button is focused (either with the mouse or with the Tab key), I get a rounded black outline. This applies to all buttons, both inside and outside button groups.

Hi @Catrope, this is how I trigger it (I get this only on Chrome v104 on my MacOS Monterey:

1. Use the Tab key to focus on a button in a group.
2. Long-click on the button.

Do let me know if you get the same experience.

@EUdoh-WMF I can't reproduce it either on Chrome/macOS Monterey. Not with tabbing in and then either Enter, Space or the touchbar. Never gets rounded as in the picture. I'd think that's an obscure OS feature, like the Dictionary lookup in Finder/Safari.

@Volker_E @Catrope This is an interesting one. So far, @bmartinezcalvo and I can see this. There is the possibility that someone else out there may come across this behaviour. I am happy to document this experience in another ticket and move this to Product Sign-Off.

This is the ticket for this behaviour - T317538.

For completion, the issue uncovered by @EUdoh-WMF is valid and actively worked on in T317538, but has no impact on this ButtonGroup (MVP) task.

product sign-off