WIP: Allow generating iconset for HASS icons by balloob · Pull Request #1100 · home-assistant/frontend

balloob · 2018-04-18T03:49:00Z

WIP until we migrate to Polymer 3

Included icons will be limited to our official icons (the ones we refer in the frontend code). Size of included icons will be reduced from 747K to 38K.

This new gulp script will:

Search through our code and find all icons that we use in our official code.
Create a new icon set called hass based on icon set mdi with only the icons that we use

To do:

We should refer to icons in our code as hass: and update our search regex
~~We should add a config option to the frontend component to allow loading the MDI icons~~
~~We should also search the python code for used icons??~~
Fork iron-icon to see if missing iconset is mdi and load mdi when missing
Update gen-index-html script
Convert download mdi script to a pure JS solution instead of a Python/JS hybrid
Get it to work. Somehow loading mdi on demand does not seem to work

Not part of this PR but would help loading time:

Migrate as much Python code in the backend to use device_class instead of icon.

houndci-bot · 2018-04-18T03:49:06Z

+  defs.childNodes = defs.childNodes.filter(icon => icons.has(icon.attrs[0].value));
+
+  fs.writeFileSync('hass_frontend/hass_icons.html', parse5.serialize(iconDoc));
+  console.log(`Home Assistant has ${icons.size} icons.`);


Unexpected console statement no-console

houndci-bot · 2018-04-18T03:49:07Z

+
+  const ironIconset = iconDoc.childNodes[0];
+  ironIconset.attrs.forEach(attr => {
+    if (attr.name == 'name') {


Expected '===' and instead saw '==' eqeqeq

houndci-bot · 2018-04-18T03:49:07Z

+    fs.readFileSync('hass_frontend/mdi.html', { encoding: 'utf-8' }));
+
+  const ironIconset = iconDoc.childNodes[0];
+  ironIconset.attrs.forEach(attr => {


Expected parentheses around arrow function argument having a body with curly braces arrow-parens

houndci-bot · 2018-04-18T03:49:07Z

+async function generateHassIcons() {
+  const icons = findIcons();
+
+  const iconDoc = parse5.parseFragment(


Unexpected newline after '(' function-paren-newline

houndci-bot · 2018-04-18T03:49:07Z

+    let match;
+    while (match = iconRegEx.exec(content)) {
+      icons.add(match[0].substr(4));
+    };


Unnecessary semicolon no-extra-semi

houndci-bot · 2018-04-18T03:49:07Z

+  function processFile(filename) {
+    const content = fs.readFileSync(filename);
+    let match;
+    while (match = iconRegEx.exec(content)) {


Unexpected assignment within a 'while' statement no-cond-assign

andrey-git · 2018-04-18T05:10:23Z

Yes, we should also search for python code.

However this makes different namespace difficult

NovapaX · 2018-04-18T07:44:52Z

We could fork iron-icon or iron-iconset and let us provide two sources for one iconset. If the primary source (i.e. our packed mdi: subset) fails, load the fallback and try there....
Although a typo in an icon-name would immediately load the fallback iconset.

the advantage of loading the full mdi: on-demand under the same namespace would be that while developing (but also users customizing) you don't need to think about the namespace to use.

balloob · 2018-04-20T01:40:23Z

What if we get rid of all icon definitions on the Python side as part of the core. Instead, icons should be based on device_class or be derived from domain. That way we don't have to search the Python code.

(people can still define icon and it would then load mdi icons on demand.)

balloob · 2018-04-20T01:49:34Z

Well, removing icons from the backend might be quite the task…

balloob · 2018-04-20T01:50:18Z

Guess defining an icon in the backed can sometimes make sense, like with Alpha Advantage:

balloob · 2018-04-20T01:53:31Z

Probably best approach would be to fork iron-icon to trigger it to load mdi icon set.

We would patch around here:

https://github.com/PolymerElements/iron-icon/blob/7f735696e8d8f6edddb16d78da7da94a3638fa08/iron-icon.html#L182-L184

houndci-bot · 2018-04-20T03:00:49Z

+  defs.childNodes = defs.childNodes.filter(icon => icons.has(icon.attrs[0].value));
+
+  fs.writeFileSync('hass_frontend/hass_icons.html', parse5.serialize(iconDoc));
+  console.log(`Home Assistant has ${icons.size} icons.`);


Unexpected console statement no-console

balloob · 2018-04-20T03:33:39Z

So I played a bit more with this. Added a new ha-icon which is a fork of iron-icon but with the code attached to load mdi.html on demand. However, this doesn't seem to work 🤔

The dynamic panels still use mdi icons. As you can see, icon for Configuration is missing. I have confirmed in the network panel that mdi.html has loaded, yet the iconset doesn't seem to get registered.

Adding some debug statements to ha-icon, I can see ha-icon event listener being triggered and it can find the icon. Yet, when it calls this._iconset.applyIcon, nothing happens.

NovapaX · 2018-04-21T20:22:58Z

Could this be related?
PolymerElements/iron-iconset-svg#77

balloob · 2018-04-23T02:41:45Z

That's totally it. When I patch iron-iconset-svg to use the PR by @andrey-git it works.

balloob · 2018-04-23T03:33:24Z

Since I'm already updating the updateIcon method of ha-icon to add mdi on-demand loading, I also introduced a little hack to detect if an iconset might not be loading yet:

b271518

It's not pretty.

Sign in to view

+  const downloadMatch = content.match('(/api/download/polymer/v1/([A-Z0-9-]{36}))');
+
+  if (!downloadMatch) {
+    console.error('Unable to find Polymer v1 download link');


Sign in to view

+const parse5 = require('parse5');
+const getContent = require('../common/http').getContent;
+
+const outputDir = 'hass_frontend';


Sign in to view

+  const downloadMatch = content.match('(/api/download/polymer/v1/([A-Z0-9-]{36}))');
+
+  if (!downloadMatch) {
+    console.error('Unable to find Polymer v1 download link');


balloob · 2018-04-30T03:10:11Z

I've converted the MDI download script to JavaScript, dropping the last Python dependency 🎉

PR on backend to convert built-in panels to use HASS iconset: home-assistant/core#14185

built-in panel icons are hardcoded in the gulp task.

Updated service worker to handle fingerprinting of hass_icons.html correctly and handle the lack of fingerprinting on mdi.html correctly.

andrey-git · 2018-04-30T14:28:15Z

(didn't look at the code yet) I think we should support icons set by platforms (in Python)

balloob · 2018-04-30T18:01:45Z

Problem with icon set per platform is the duplication that might occur when we introduce even more namespaces. We currently have overlap between hass and mdi namespace, but we're assuming that the mdi namespace might not be necessary for basic users.

I prefer if we first see how far sensor device classes is going to get us before we invest time in making icon sets for Python code. In a perfect world, the Python code should not be aware of icons, although I do realize that it's better for us to break that rule here to make the simple things not overly complicated.

c727 · 2018-05-02T14:01:02Z

 const dynamicUrlToDependencies = {};

 const staticFingerprinted = [
-  'mdi.html',


we still have to fingerprint mdi

No. It's only going to be loaded from inside ha-icon and that will reference to it without fingerprint.

When a new service worker is installed, it will check the fingerprint and download a new version if it has changed.

balloob · 2018-06-07T18:39:53Z

This has been fixed in #1214.

homeassistant added the cla-signed label Apr 18, 2018

houndci-bot reviewed Apr 18, 2018

View reviewed changes

houndci-bot reviewed Apr 20, 2018

View reviewed changes

balloob force-pushed the icon-subset branch from 429db8e to 03a8711 Compare April 20, 2018 03:24

homeassistant added the cla-signed label Apr 20, 2018

This was referenced Apr 21, 2018

Add Sensor device_class support #1115

Merged

Sensor device class docs home-assistant/home-assistant.io#5219

Merged

homeassistant added the cla-signed label Apr 23, 2018

balloob force-pushed the icon-subset branch from b271518 to ba43d7a Compare April 27, 2018 14:27

homeassistant added the cla-signed label Apr 27, 2018

balloob added 6 commits April 29, 2018 21:52

Allow generating iconset for HASS icons

66b80a0

Lint

343cca8

Load mdi on demand

fd73167

mdi -> hass

1d29291

Lint

892e33a

Hack reschedule

39dfdfb

balloob force-pushed the icon-subset branch from ba43d7a to 39dfdfb Compare April 30, 2018 01:53

homeassistant added the cla-signed label Apr 30, 2018

balloob added 2 commits April 29, 2018 21:56

Revert mdi -> hass change for customize

7d1dc1e

Clarify on top of ha-icon what has changed

24e8df9

balloob added 2 commits April 29, 2018 22:29

Convert latest mdi icon references

a9026f7

Convert python script to JS

004ef4c

houndci-bot reviewed Apr 30, 2018

View reviewed changes

balloob added 2 commits April 29, 2018 22:32

BUILD_DEV=0 all the things

4458501

Embed built-in panel icons

69f22d5

houndci-bot reviewed Apr 30, 2018

View reviewed changes

Comment thread gulp/tasks/hass-icons.js

const downloadMatch = content.match('(/api/download/polymer/v1/([A-Z0-9-]{36}))');

if (!downloadMatch) {

console.error('Unable to find Polymer v1 download link');

This comment was marked as resolved.

Sign in to view

balloob added 2 commits April 29, 2018 23:04

service worker hass icons fix

408cede

Lint

7e46d4e

balloob mentioned this pull request Apr 30, 2018

Use hass iconset home-assistant/core#14185

Merged

c727 reviewed May 2, 2018

View reviewed changes

balloob changed the title ~~Allow generating iconset for HASS icons~~ WIP: Allow generating iconset for HASS icons May 3, 2018

This was referenced May 26, 2018

Load icons via JS #1211

Merged

Generate Hass and Hass.io specific iconset #1214

Merged

balloob closed this Jun 7, 2018

balloob deleted the icon-subset branch June 7, 2018 18:39

github-actions Bot locked and limited conversation to collaborators Jul 8, 2022

Conversation

balloob commented Apr 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

WIP until we migrate to Polymer 3

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrey-git commented Apr 18, 2018

Uh oh!

NovapaX commented Apr 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

balloob commented Apr 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

balloob commented Apr 20, 2018

Uh oh!

balloob commented Apr 20, 2018

Uh oh!

balloob commented Apr 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

balloob commented Apr 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NovapaX commented Apr 21, 2018

Uh oh!

balloob commented Apr 23, 2018

Uh oh!

balloob commented Apr 23, 2018

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

balloob commented Apr 30, 2018

Uh oh!

andrey-git commented Apr 30, 2018

Uh oh!

balloob commented Apr 30, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

balloob commented Jun 7, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

balloob commented Apr 18, 2018 •

edited

Loading

NovapaX commented Apr 18, 2018 •

edited

Loading

balloob commented Apr 20, 2018 •

edited

Loading

balloob commented Apr 20, 2018 •

edited

Loading

balloob commented Apr 20, 2018 •

edited

Loading