Per #3111409: Add new Olivero frontend theme to Drupal 9.1 core as beta, we need to ensure that all SVGs are optimized.

After optimizing, we need to test in all browsers including IE11.

CommentFileSizeAuthor
#7 interdiff_5-7.txt2.69 KBkostyashupenko
#7 3165449-7.patch45.8 KBkostyashupenko
#5 3165449-5.patch44.82 KBkostyashupenko
#3 3165449-3.patch40.72 KBkostyashupenko

Comments

mherchel created an issue. See original summary.

kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented
Assigned: Unassigned » kostyashupenko
kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented
Assigned: kostyashupenko » Unassigned
Status: Active » Needs review
StatusFileSize
new 3165449-3.patch40.72 KB

1. Optimized all svg icons (simplified, compressed)
2. Added throbber.gif for autocomplete-loading inputs for IE browser
3. Reworked twig templates: removed all inline svg and replaced by construction {% include "@olivero......svg" %}

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Needs work

I'm getting an error when applying the patch:
error: cannot apply binary patch to 'images/throbber.gif' without full index line

Did you use the --binary option when creating the patch?

kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented
Status: Needs work » Needs review
StatusFileSize
new 3165449-5.patch44.82 KB
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Needs work

This is looking really good! I found two regressions, one somewhat major, and one really minor!

+++ b/css/components/messages.pcss.css
@@ -112,23 +112,23 @@
+.messages__icon svg {

These changes break the color of the messages SVG. The SVG appears black on an almost-black background, so it cannot be seen. Looking at the DOM, the SVG is the element that has the .messages__icon CSS class.

+++ b/css/components/autocomplete-loading.module.pcss.css
@@ -54,3 +56,58 @@
+    var(--autocomplete-throbber-IE-icon-url) no-repeat right var(--autocomplete-icon-right-offset) top var(--autocomplete-icon-top-offset) / var(--autocomplete-throbber-IE-size),

When I tested out the autocomplete in IE11, the throbber was loading on the left side of the input (when it should have been on the right side of the input - replacing the search icon). This is obviously super minor, so if it's a pain to fix properly, we can split out into another issue.

kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented
Status: Needs work » Needs review
StatusFileSize
new 3165449-7.patch45.8 KB
new interdiff_5-7.txt2.69 KB

Hello @mherchel,
1. i can't reproduce

These changes break the color of the messages SVG. The SVG appears black on an almost-black background, so it cannot be seen. Looking at the DOM, the SVG is the element that has the messages__icon CSS class

Maybe you didn't clear cache? Since there is some changes in status-messages.html.twig

2.

When I tested out the autocomplete in IE11, the throbber was loading on the left side of the input (when it should have been on the right side of the input - replacing the search icon). This is obviously super minor, so if it's a pain to fix properly, we can split out into another issue.

Should be fine now

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Reviewed & tested by the community

Hmmm... maybe it was a caching issue. I can't reproduce anymore :)

Looks good!

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Reviewed & tested by the community » Fixed

Committed!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.