Closed (fixed)
Project:
Drupal core
Version:
9.2.x-dev
Component:
Olivero theme
Priority:
Major
Category:
Task
Assigned:
Issue tags:
Reporter:
Created:
23 Jan 2021 at 13:54 UTC
Updated:
20 May 2021 at 18:53 UTC
Jump to comment: Most recent, Most recent file


Comments
Comment #2
mherchelSome initial work on this. Still not done.
Comment #3
mherchelComment #4
mherchelPatch attached. This needs review!
A tugboat preview can be found at https://3194350-forms-2-uxdbja8p1vj23ttknnwraqkvaevzpbq2.tugboat.qa/
Note that there are webform and style guide example forms.
Comment #5
mherchelUpdated patch fixes the autocomplete icon.
Comment #6
mherchelVideos attached. Note that videos can't quite show everything, so please look around the Tugboat preview at https://3194350-forms-2-uxdbja8p1vj23ttknnwraqkvaevzpbq2.tugboat.qa/
Comment #7
mherchelFound an issue on Chrome on Android where the browser was defaulting to a gray background for
selectandinput[type="date"]elements.Fix attached.
Comment #8
oksana-c commentedthe new form design looks slick! and so easy to preview with Tugboat.
I noticed that the number field has width set to 4em and if the max for field is a two or three digit number, then the second and third digit is cut off. Adding screenshots. It'd be best to keep the width of the field at `auto`.
number field with set width
number field with auto width
I haven't dug through any source code and not certain if the 4em value comes from Olivero, so may not be in scope here.
Comment #9
mherchelGood catch. That is out of scope, because those styles are being added by the webform module. Thanks for the review!
Comment #10
mherchelThis latest patch removes some old CSS variables that were not transpiling (because they didn't exist).
Comment #11
mherchelGave @andrewmacpherson a walk though on this yesterday. He didn't see any major issues, although it definitely needs more thorough testing.
Comment #12
andrewmacpherson commentedSorry for being slow to RTBC this one. Confirming comment #11 - @mherchel and I did walk through the implementation during a video call.
I didn't see any significant deviations from the agreed designs from the earlier issue, so the implementation (broadly) looks good to go. We did note that in Windows high-contrast mode, a few things could do with some extra polish for the sake of elegance, but I don't think we saw any blockers there.
So I recommend we commit the work done so far, and if any glitches or accessibility issues are noticed afterwards they can have more specific issues. What we have here is several big steps forward.
Comment #13
mherchelYay!
Some minor code cleanup changes in this patch.
Comment #14
mherchelForgot to transpile the CSS 🤪
New patch and interdiff attached.
Comment #16
alexpottI reviewed this with @lauriii - we took it for a test drive and it always lovely to see a see diff stat
31 files changed, 326 insertions, 1584 deletions.yum.The focus state of inputs does quite match the design. There are rounded corners on the design but with the patch there aren't and the elements also get unrounded corners so it looks a bit odd.
Also we couldn't decide if the check in a checkbox was correctly aligned - not sure that it is as central as it should be:
Comment #17
lauriiiI noticed that the dropbutton elements can be hard to distinguish from each other when the tray is open because it doesn't have any border. See this gif for an example of that:

Comment #18
mherchelGood catch on the drop buttons!
I reviewed the focus designs with @jwitkowski79, and she signed off on the focus state issue. The gist of the matter is that outlines do not support rounded corners (unless they're set to auto, but then you cannot change their color in FF & Safari). To replicate the rounded corners using borders and box shadows can cause layout shifts (which outlines do not do), and/or be invisible in Windows High Contrast mode (box shadows).
I'll get her to comment in here. If you don't like them, we can create a followup issue.
Comment #19
alexpott@mherchel re focus styles - if both you and @jwitkowski79 are happy I'm happy! I had to comment because all I had to go on were the designs in the issue summary and figma and the patch itself. If there are other areas where the implementation and the design don't match it'd be great to detail them.
Comment #20
jwitkowski79 commentedHello! Mike did indeed review the focus states with me and we discussed different options. Removing the rounded corners was the best way forward. I give my thumbs up!
Comment #21
mherchelUpdated patch attached adding borders to the dropbutton.
Notes:
Image:
Comment #22
ressaThanks @mherchel, I tested the new patch with latest Drupal 9.2.0-dev in Firefox, Chromium, and GNOME Web browsers (OS: Ubuntu) and it looks great, and just as in your dropbutton.gif.
Comment #23
alexpottIt doesn't look so great on the content types page see below...
Also the way the light blue bleeds over the drop button separator looks odd.
Comment #24
mherchelyeah, tbh, we could completely re-write those drop buttons to be more in-line with what Claro has done.
Would you be okay committing this and creating a stable-blocking followup issue on that? I feel that I want to get this in sooner than later so to make it easier to do additional testing.
Comment #25
mherchelI asked @alexpott and @lauriii in Slack if we can break out the drop-buttons into another issue and they gave the 👍
Opened up #3200370: Fix Olivero's drop-button style to conform with new form styles
Comment #26
mherchelUpdated patch with minor change that resolves the checkbox centering issue that @alexpott mentioned in #16.
The change in the styling is below in this animated gif
Comment #27
mherchelUpdated Tugboat preview is at https://3194350-forms-3-i6jx1gsqm8ja3pueb9ggwihq1hmpdbll.tugboat.qa/form...
Comment #28
katannshaw commentedJust reviewed patch #26. LGTM. Marking RTBC.
Comment #29
mherchelCreated followup #3200628: Olivero's small button variation's text seem vertically mis-aligned
Comment #30
mherchelNew followup: #3200631: Olivero: jQuery UI dialog's select elements do not contain dropdown chevron in Safari
Comment #31
mherchelCreated followup #3200642: Olivero's select chevron icon doesn't use the "high contrast" color in high contrast mode
Comment #32
mherchelSpawned #3200644: Olivero's autocomplete input does not have disabled styling
Comment #33
mherchelComment #34
lauriiiI reviewed the code and did some cross-browser compatibility testing with @mherchel. @mherchel has opened some follow-ups for most of the issues discovered so we could land this issue as soon as soon as possible.
I did find one nitpick on the code. This should probably use
--border-radiusvariable.Comment #35
mherchelResolving #34
Note that I did compile the CSS with this patch, but there is no change within the compiled CSS.
Comment #37
mherchelSetting back to RTBC. The last test failure was unrelated, and the newest test pass passed.
Comment #38
lauriiiCommitted 03bf48d and pushed to 9.2.x. Thanks!
Comment #41
gábor hojtsy