Broken out from issue #3194350: Implement new form element designs for Olivero. We need some additional work on the drop-buttons.
Ideally, I'd like to copy what Claro has done (and refactor it as needed).
We also need to ensure that the drop-button's various focus styles are highly visible (they currently are not)

| Comment | File | Size | Author |
|---|---|---|---|
| #42 | interdiff-40-42.txt | 1017 bytes | mherchel |
| #42 | 3200370-42.patch | 12.25 KB | mherchel |
| #41 | 3200370-single-dropdown.png | 9.19 KB | indrajithkb |
| #40 | interdiff-37-40.txt | 982 bytes | mherchel |
| #40 | 3200370-40.patch | 12.21 KB | mherchel |
Comments
Comment #2
mherchelComment #3
mherchelComment #4
mherchelPatch attached.
Here's what it looks like. I also attached a movie showing multiple browsers and high contrast mode.
Comment #5
gauravvvv commented#4, Looks fine to me. RTBC +1
Comment #6
abhijith s commentedApplied patch #4 and it works fine.
Before patch:

After patch:

RTBC +1
Comment #7
mherchelThanks for the review! Don't forget to change the status to RTBC when you RTBC issues :D
Comment #8
mherchelFound a bug in this. It doesn't account for the single variant of the dropbutton.

Comment #9
imalabya@mherchel Please review the patch which addresses the single variant. Attached screenshots.
Comment #10
imalabyaUpdated patch
Comment #11
mherchelIt looks like you added the patch to the patch :D (patchception!)
I'm attaching an updated patch. I removed the
width: auto, and also added some RTL fixes.Comment #12
katannshaw commentedLooks and works great. Marking RTBC!
Comment #13
mherchelOpened followup #3201889: Olivero's dropbuttons expand to full width when not inside of table in IE11
Comment #14
mherchelTalking with @lauriii in slack and he had a comment on the z-indexes, but didn't get a chance to post it. We need to make sure we're following https://www.drupal.org/docs/theming-drupal/z-indexes-in-drupal-8
Comment #15
mherchelUpdating patch by setting the z-index for open dropbuttons to 50 per Drupal's z-index documentation.
I also fixed an IE11 issue where the dropbuttons were expanding to full width when outside of tables.
Comment #16
proeungPatch #15 look good and works great. Marking this RTBC!
Comment #17
kleiton_rodrigues commentedWorks good! Moving to RTBC +1!
Comment #18
lauriiiFew things I noticed while testing this:
The text on the button appears to be off-center.
Comment #19
mherchelFixed this by removing the line-height.
Fixed with an outline-offset.
Comment #20
bhumikavarshney commentedHi @mherchel
#19 patch Works good for me! Moving to RTBC +1!
Comment #21
ressaThe button text is now aligned, and it looks great:
Comment #22
mherchelCreated followup #3209532: Olivero: focus style on the dropbutton toggle button is not properly aligned with the focus style of adjacent link
Comment #23
alexpottI think there is a grey line appearing under the dropbutton in the down state that is not meant to be there. See:

Comment #24
alexpottThis extra line is cause by the
border: solid 1px transparent;inwithout that line the drop buttom looks way sharper.
Comment #25
kapilv commentedAddress #24.
Comment #26
mherchel@KapilV Thanks for the patch, but you'll need to 1) ensure that you're working on the pcss.css version of the file and then compile the CSS version. You'll also want to find out why the transparent border was added and account for that use case.
I'm working on this right now.
Comment #27
mherchelWe can't get rid of the transparent border because it's needed by Windows High Contrast users. That being said, I was able to fix the underlying issue by moving the background color from the container to the list item.
patch and interdiff attached.
Comment #28
chetanbharambe commentedComment #29
chetanbharambe commentedVerified and tested patch #27.
Patch applied successfully and looks good to me.
Testing Steps:
# Set the Olivero theme from Appearance.
# Goto: /admin/structure/types/manage/article/fields
# User should see dropdown-button style to conform with new form styles.
Looks good to me.
Can be a move to RTBC
Please refer attached screenshots for Before and After patch.
Comment #32
catchRestoring status after HEAD was broken.
Comment #33
mherchelCreated followup to fix text vertical alignment at #3212828: Olivero: Text within drop-button not vertically aligned
Comment #34
lauriiiBased on https://www.drupal.org/docs/theming-drupal/z-indexes-in-drupal-8, shouldn't this be 100?
Comment #35
mherchelYou are correct! Updated patch attached. Leaving RTBC as this is a very minor change.
Comment #36
lauriiiLong texts are not legible in browsers that don't support max-content
Dropbuttons with just a single item are missing border radius on the right
Comment #37
mherchelFixed the issue from #36. Note to test this, you must restrict the width of the parent
.dropbutton-wrapper, and then edit the text of the action so it's very long.Comment #38
indrajithkb commentedHi @mherchel the second point is still not fixed from #36 by @lauriii
1. Still we missed the border-radius

Attaching SS:
2. When test is long

SS after patch for long text (without limitting the wrapper width).
3. Here we couldn't see the separation between the two dropdowns

SS after patch for long text (with limiting the wrapper width).
All these text is edited through browser dev-tools.
Attaching the SS of long dropdown item before patch (might be helpful for finding the 3rd point):
Comment #39
mherchelGood catch on the border radius. I completely missed that comment.
I'm not too worried about your point in #3. I'm not sure where the drop-button is going to be width constrained, and even if so, I'm not quite sure how we'd get to the point where the text gets long enough for it to wrap. We're getting into some very edge cases here, and if it does become a problem, I'd rather open up a followup issue.
I appreciate the thorough review, though!
Comment #40
mherchelHere is a patch that adds back in the border-radius when it has a single action.
Comment #41
indrajithkb commentedHi @mherchel thanks for the patch, now it's working as expected.
Attaching screenshot after patch #40

This was remaining task on this issue , now it's get resolved. So am moving the issue to RTBC.
Comment #42
mherchelOne minor change: Adding
cursor: pointeronto the button element.Comment #43
lauriiiCommitted 61ebd08 and pushed to 9.3.x. Thanks!