Icon font to SVG migration part 1 #6107 #10277

thibaudcolas · 2023-03-28T15:31:18Z

Assorted icon font -> SVG conversions as part of #6107. This is missing a few areas of the CMS still, which will come in a "part 2" (#10278). In particular workflows, ModelAdmin, and listings will be done in a separate PR.

This might be easier to review commit-by-commit – I’ll add comments directly in the PR for specific changes. At a high level:

Icons have been switched to inline SVGs where possible, and SVG mask-image where inline SVGs would be impractical.
Some icons will render with different alignment after this change. I’ve decided to not adjust styles here, as we are revising the visuals of almost all icons in Wagtail icon set should be extended and improved #2349, which will involve further alignment changes anyway.
There should be no backwards compatibility issues with those changes.

Do the tests still pass?[^1]
Does the code comply with the style guide?
~~[ ] For Python changes: Have you added tests to cover the new/fixed behaviour?~~
For front-end changes: Did you test on all of Wagtail’s supported environments?[^2]
- Please list the exact browser and operating system versions you tested: latest Chrome Safari Firefox on macOS 13.2
- Please list which assistive technologies [^3] you tested: None
~~[ ] For new features: Has the documentation been updated accordingly?~~

squash-labs · 2023-03-28T15:31:26Z

Manage this branch in Squash

Test this branch here: https://thibaudcolas6107-icon-font-par-654r4.squash.io

thibaudcolas · 2023-03-28T15:32:03Z

client/scss/components/_dropdown.scss

-  &:before {
-    display: none; // TODO: remove when iconfont styles are removed
-  }
-


All dropdown chevron icons are already converted so this doesn’t need to wait.

thibaudcolas · 2023-03-28T15:33:42Z

client/scss/components/_icons.scss

+    width: 1em;
+    height: 1em;
+    display: inline-block;
+    background-color: currentColor;


This results in the icons looking more or less the same as before. I chose to keep those three "-after" icons done via CSS for two reasons:

So this keeps working for people implementing listings by copy-pasting Wagtail’s own listings.

So we introduce appropriate listing components in Universal listings roadmap#33

thibaudcolas · 2023-03-28T15:35:14Z

client/scss/components/_icons.scss

-    color: $color-grey-2;
-    border: 1px solid currentColor;
-  }
-}


This wasn’t used, and wasn’t documented in our list of available icons.

thibaudcolas · 2023-03-28T15:35:56Z

client/scss/components/_indicator.scss

-    &:before {
-      font-size: 1.5em;
-    }
-  }


Icon size defined via a reusable icon-specific class.

thibaudcolas · 2023-03-28T15:36:57Z

client/scss/components/forms/_error-message.scss

    vertical-align: -10%;
-    content: map.get($icons, 'warning');
+    mask-image: url('#{$images-root}icons/warning.svg');
+    background-color: currentColor;


Same as -after icons – a lot of people have "error-message" directly referenced in their templates, and we don’t have a reusable component for this, so keeping the icon in CSS for now.

Should we remove that line above while we're in this code.

// UI-Redesign: to be added via js and styled here

And maybe the Todo note?

Hmm I don’t understand what it’s about so sounds sensible to me.

I assume it's about how error message elements for StreamField are constructed? If I remember correctly, they're dynamically created on the client-side via JS as opposed to server-rendered.

thibaudcolas · 2023-03-28T15:38:12Z

client/src/components/StreamField/blocks/ListBlock.test.js

@@ -113,7 +113,7 @@ describe('telepath: wagtail.blocks.ListBlock', () => {
        icon: 'placeholder',
        classname: null,
        helpText: 'use <strong>a few</strong> of these',
-        helpIcon: '<div class="icon-help">?</div>',
+        helpIcon: '<svg></svg>',


Outside of tests, helpIcon renders an SVG icon since quite a few releases ago. I chose to use an empty <svg> element as the exact markup is irrelevant here.

thibaudcolas · 2023-03-28T15:39:35Z

client/src/entrypoints/admin/core.js

@@ -308,11 +308,8 @@ if (!wagtail.ui) {
 const DROPDOWN_SELECTOR = '[data-dropdown]';
 const LISTING_TITLE_SELECTOR = '[data-listing-page-title]';
 const LISTING_ACTIVE_CLASS = 'listing__item--active';
-const ICON_DOWN = 'icon-arrow-down';
-const ICON_UP = 'icon-arrow-up';
 const IS_OPEN = 'is-open';


This was all dead code. Dropdown up/down chevrons had already been rewritten to use the is-open class only to determine which icon to show.

thibaudcolas · 2023-03-28T15:40:50Z

wagtail/admin/templates/wagtailadmin/pages/history.html

@@ -4,7 +4,7 @@
 {% block titletag %}{% blocktrans trimmed with title=subtitle %}Page history for {{ subtitle }}{% endblocktrans %}{% endblock %}

 {% block actions %}
-    <a href="{% url 'wagtailadmin_pages:edit' page.id %}" class="button bicolor icon icon-edit">{% trans "Edit this page" %}</a>
+    <a href="{% url 'wagtailadmin_pages:edit' page.id %}" class="button bicolor button--icon">{% icon name="edit" wrapped=1 %}{% trans "Edit this page" %}</a>


Note on wrapped=1 – not the nicest API but essentially this is needed for button--icon buttons. The buttons look bigger like this than with the icon font, which we’ve accepted in other parts of the UI.

thibaudcolas · 2023-03-28T15:58:12Z

client/scss/components/_listing.scss

-        height: 100%;
-        vertical-align: middle;
-      }
-    }


Those have been replaced with the same -after ordering icons as other listings.

thibaudcolas · 2023-03-28T15:59:05Z

wagtail/users/templates/wagtailusers/users/create.html

@@ -44,8 +44,9 @@
                        {% endblock fields %}

                        <li>
-                            <a href="#tab-roles" data-tab-trigger class="button lowpriority icon icon-arrow-right-after">


I couldn’t find a reference to lowpriority anywhere else.

lb-

Did a smoke test of all icon scenarios changed - did not see any issues. One minor comment.

Happy to approve but I see there are some CI items to fix still.

lb- · 2023-03-29T07:49:19Z

client/scss/components/forms/_error-message.scss

    vertical-align: -10%;
-    content: map.get($icons, 'warning');
+    mask-image: url('#{$images-root}icons/warning.svg');
+    background-color: currentColor;


Should we remove that line above while we're in this code.

// UI-Redesign: to be added via js and styled here

And maybe the Todo note?

thibaudcolas · 2023-03-29T08:17:59Z

Thank you @lb-! The CI failures are just because I’ve cancelled the GitHub Actions builds, it’s all green in CircleCI (which is all that’s relevant for this kind of change)

…ther listings

thibaudcolas commented Mar 28, 2023

View reviewed changes

thibaudcolas mentioned this pull request Mar 28, 2023

Icon font to SVG migration part 2 #6107 #10277 #10278

Merged

5 tasks

thibaudcolas commented Mar 28, 2023

View reviewed changes

thibaudcolas added this to the 5.0 milestone Mar 28, 2023

thibaudcolas added type:Cleanup/Optimisation component:Icons status:Needs Review labels Mar 28, 2023

thibaudcolas self-assigned this Mar 28, 2023

thibaudcolas marked this pull request as ready for review March 28, 2023 16:00

thibaudcolas requested a review from laymonage March 28, 2023 16:00

lb- reviewed Mar 29, 2023

View reviewed changes

lb- mentioned this pull request Mar 29, 2023

Fix issue with MessagesController (base html) - error type #10281

Merged

laymonage approved these changes Mar 29, 2023

View reviewed changes

laymonage removed the status:Needs Review label Mar 29, 2023

thibaudcolas added 14 commits March 29, 2023 13:17

Remove unneeded icon-help definitions

a84280c

Remove unused help-inverse icon

5f79725

Convert privacy switch to SVG icons

a877e44

Convert image and document FileFields to SVG icons

be2595b

Convert history edit buttons to SVG icons

82f6954

Convert error messages to SVG icons

bb7638c

Convert jQuery Datetimepicker to SVG icons

6e11f54

Convert "after" ordering icons to SVG

cd2dd28

Convert add multiple thumbnail icon to SVG

876dfc5

Remove dead styles from legacy dropdown

d370c26

Fix typo in header.html docs

72440fa

Convert form submissions listings to use the same ordering icons as o…

b790bdb

…ther listings

Convert user creation form to SVG icons

2bd9cbf

Convert "add child page" icon to SVG

ad0d715

Release notes for wagtail#10277

ad26370

thibaudcolas force-pushed the 6107-icon-font-part-1 branch from caf7eb0 to ad26370 Compare March 29, 2023 12:20

thibaudcolas merged commit b504403 into wagtail:main Mar 29, 2023
4 of 7 checks passed

thibaudcolas deleted the 6107-icon-font-part-1 branch March 29, 2023 12:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Icon font to SVG migration part 1 #6107 #10277

Icon font to SVG migration part 1 #6107 #10277

thibaudcolas commented Mar 28, 2023 •

edited

squash-labs bot commented Mar 28, 2023

thibaudcolas Mar 28, 2023

thibaudcolas Mar 28, 2023

thibaudcolas Mar 28, 2023

thibaudcolas Mar 28, 2023

thibaudcolas Mar 28, 2023

lb- Mar 29, 2023

thibaudcolas Mar 29, 2023

laymonage Mar 29, 2023

thibaudcolas Mar 28, 2023

thibaudcolas Mar 28, 2023

thibaudcolas Mar 28, 2023

thibaudcolas Mar 28, 2023

thibaudcolas Mar 28, 2023

lb- left a comment

lb- Mar 29, 2023

thibaudcolas commented Mar 29, 2023

Icon font to SVG migration part 1 #6107 #10277

Icon font to SVG migration part 1 #6107 #10277

Conversation

thibaudcolas commented Mar 28, 2023 • edited

squash-labs bot commented Mar 28, 2023

Manage this branch in Squash

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lb- left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thibaudcolas commented Mar 29, 2023

thibaudcolas commented Mar 28, 2023 •

edited