-
-
Notifications
You must be signed in to change notification settings - Fork 429
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Item - Category field #3052
Comments
@ghys Sounds like an UI issue. Can you take care of that? |
Yep although we might need to redefine what an item's "category" means, both in core and in the UI. |
See also #3173 which also shows that there is an issue with the differentiation between category and icon. |
This come in recent 4.0 wishlist topic (forum is down atm, can't link it), so I'd consider it relevant for next release. Given that recent docs mention this section as icon, so my proposal is to relax validation of that field to match main UI syntax and also address that in icon lookups making i.e. |
This issue has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/openhab-4-0-wishlist/142388/393 |
I started to implement iconify support in BasicUI. MainUI will support all these icon sources. Here are valid examples:
Is it ok for everybody ? I think this is exactly the same syntax that was defined in MainUI for oh-icon. @ghys : please confirm. |
Personally I have no objections to syntax itself, if it matches assumptions made earlier by UI does its. As a side note - UI does not ship image files but font files which means that use of them is generally speaking done through proper manipulation of CSS, will it work with other clients such as ios or android? |
These app will also have to be enhanced. |
Sorry, but this means that two more places will need extra logic around rendering these icons while they could be using iconset delivered by core. Making it so would allow to reduce logic in all clients and simply delegate work to |
The icon servlet implemented in core framework was designed to return an image (like PNG or SVG). @ghys : please help me to explain why OH core icon provider/servlet is not adapted to these specific icon sources, in case I was not enough precise.
Isn't it what we are discussing here ? |
I had look at this and iconify has each icon in SVG format. The UI provider I linked in forum post embeds bunch of these svgs and make them available as svg. Its takin a lot of space hence I will look if its possible to tranform webfonts/fonts into svg on the fly to reduce size of binary distribution. |
The framework7 icons and Material icons are available in SVG format resp. here and here, so they could be made into regular iconsets. If their size is a concern they could be distributed via the Marketplace, so users can install the bundle on-demand if they need them in BasicUI/sitemaps instead of being bundled in the distribution.
I think framework7 and openhab aren't available, only f7 and oh (not a problem to have more), otherwise LGTM. |
I removed openhab and framework7. |
We have to consider that most users install OH on a RPI and the size is then critical. The option to have all these big libraries locally on the server looks relatively inappropriate especially when a user will only use few icons. By the way, if someone wants to provide new OH iconsets, this will be also supported. |
I understand your concern, however some iconsets are already included in mainui. If they are are shifted in similar form/size to core then they can be safely removed from mainui, keeping net size of assembly the same. |
For iconify, yes I accept that the icons will be fetched by browser from internet. This will not be each time because the downloaded icons will then be cached by the browser. Usage of iconify is controlled by an option. If users don't want any internet access by UI, they can disable the option. Google material font was already provided/packaged with BasicUI. So I have nothing to add (neither font nor SVG), I just allow users to use them. By the way, as I previously said, if new OH iconsets are made available in the future through the OH icon servlet, they could also be used in any existing UI including BasicUI (small change was required for BasicUI and is now done in my PR). |
I don't really like the idea of Basic UI doing requests to fetch icons from the internet for two reasons:
|
This is only an option, disabled by default. Defining other iconsets is another solution but the size of iconify library is not suitable with a small machine with not a lot of memory, like a RPI. I like my option, no need to install any additional big iconset library in the server. And please note that MainUI does not rely on server iconset to render iconify icons too. |
Are you concerned about memory as in RAM or as in persistent storage? There's a binding that add several icon sets: https://community.openhab.org/t/iconify-icon-provider/144508
Something I don't like either, but that's a bit different. Basic UI renders Sitemaps, so people will soon create feature request for the iOS and Android app as well and then we have to implement it there as well. |
Provider you linked does use iconify json files as input at build time to createe individual svg files. The end result is a fat jar which is about 30 megs. However at runtime only small part of this amount is being kept in memory. During bootatrap provider scans files generated during build time and index them. Retrieval of icon is then very effective cause we know if icon is known, and its contents is simply streamed from jar, without storing it in memory or copying over disk. Its literally same as for webui. 30 megs for todays SD cards is not much. I agree its 1/4 of openHAB download size, however almost all of that are graphical resources. They will always cause increase of size. I think influxdb which sometimes gets deployed on raspberry takes much more disk space for work than this provider. I had a look at generation of SVG from icon fonts (glyphs) and it is possible at certain effort. I have found code which can parse ttf font and generate valid svg font, however input files do not seem to have icon to character map. This seem to be an additional input. Anyhow, given the complexity of on-the-fly generation I think basic approach is still fine. ;) |
I'll gladly accept a 30 megabytes increase if the items come all out of the box. It's basically nothing. |
The icon value can now contain until 3 segments separated by a semi-column. First segment is the icon source. Example: oh, if, iconify, material, ... Second segment is the icon set (and can be empty). Example: classic Third segment is the icon name (and can contain hyphen). Example: temperature In case only two segments are provided, the icon source is assumed to be the openHAB server and the first segment is then the icon set and the second the icon name. In case only one segment is provided, the icon source is assumed to be the openHAB server and its classic icon set and the value is then the icon name. Ability to use a string containing anything has been removed. Related to openhab#3052 Signed-off-by: Laurent Garnier <[email protected]>
I tested how MainUI behaves with PR #3539, the new syntax is already partially supported. classic:humidity is working: iconify:wi:day-sunny-overcast is partially working: if:wi:day-thunderstorm is partially working: oh:classic:pressure is not working: material::favorite is not working but I am not sure if MainUI already supports Material icon source ? Or maybe the problem is the empty inconset ? @ghys and @florian-h05 for discussing that. Apparently if/iconify is not working in the item page but working in the other places. |
Here is a copy/paste of my proposal implemented in #3539 for items and #3378 for sitemap widgets.
|
That being said, that is just a question of choice. If you all prefer using material:favorite and f7:battery, then we can decide that when we have only 2 segments, the first segment is the icon source. I can then update my 2 core PRs and adjust Basic UI (very small change). This will avoid changing Main UI. |
The icon value can now contain until 3 segments separated by a semi-column. First segment is the icon source. Example: oh, if, iconify, material, f7, ... Second segment is the icon set. Example: classic Third segment is the icon name (and can contain hyphen). Example: temperature In case only two segments are provided, the first segment is the icon source and the second the icon name. "classic" icon set is assumed if icon source is "oh". In case only one segment is provided, the icon source is assumed to be the openHAB server and its classic icon set and the value is then the icon name. Ability to use a string containing anything has been removed. Related to openhab#3052 Signed-off-by: Laurent Garnier <[email protected]>
I finally changed the expectation when the icon value contains only 2 segments to be more compliant with Main UI syntax. The icon value can now contain until 3 segments separated by a colon (":"). In case only two segments are provided, the first segment is the icon source and the second the icon name. "classic" icon set is assumed. The results in MainUI:
@openhab/webui-maintainers: So I see two problems to fix in Main UI:
|
I also tested the rendering in the settings/model MainUI page, there is n surprise:
I also tested in the equipment tab of MainUI (opening the "Web services" pop-up), the place where it is the most important and it looks like it was not implemented at all.
Tested with these items:
@openhab/webui-maintainers : there is something to fix in MainUI. |
In the updated syntax, I released the restriction to not have a hyphen in the icon name. This is because most of iconify icons have one or several hyphens in their names. The OH icon server supports two different syntax for requesting an icon:
In the first way, "-" is a separator to extract the icon name and the state. So there might be a possible mismatch in case the icon name now contains "-". |
Android also does option 2 only. |
Related to openhab/openhab-core#3052 Depends on the merge of PR openhab/openhab-core#3539 Signed-off-by: Laurent Garnier <[email protected]>
Related to openhab/openhab-core#3052 Depends on the merge of PR openhab/openhab-core#3539 Signed-off-by: Laurent Garnier <[email protected]>
Related to openhab/openhab-core#3052 Depends on the merge of PR openhab/openhab-core#3539 Signed-off-by: Laurent Garnier <[email protected]>
Code in both related PR LGTM, but if I have three questions:
|
Iconsets, as previously, can be delivered by openHAB through its icon server. It then depends on the availability of new specific OH iconsets. Nothing has changed here. In parallel, MainUI and BaiscUI already implemented alternative access to other icons that are not provided by the openHAB icon server, that is: iconify icons (MainUI and BasicUI), mateiral icons (MainUI and BasicUI) and framework7 icons (MainUI only - I will study the integration in BasicUI, this is apparently more or less just a font to add).
In BasicUI, there is an option you have to enable if you want to have access to iconify icons as it requires an Internet access to an external server. Material icons do not require an access to an external server, this is just a font that is (and was already) packaged with the app. I don't know for Framework7 icons in MainUI. But @ghys could tell us. Might be just another font packaged with MainUI.
I guess iOS app was not yet enhanced to support some of the new icon features. But I have no clear idea who is maintaining the iOS app. |
The icon value can now contain until 3 segments separated by a semi-column. First segment is the icon source. Example: oh, if, iconify, material, f7, ... Second segment is the icon set. Example: classic Third segment is the icon name (and can contain hyphen). Example: temperature In case only two segments are provided, the first segment is the icon source and the second the icon name. "classic" icon set is assumed if icon source is "oh". In case only one segment is provided, the icon source is assumed to be the openHAB server and its classic icon set and the value is then the icon name. Ability to use a string containing anything has been removed. Related to #3052 Signed-off-by: Laurent Garnier <[email protected]>
I guess we can close this now, correct? |
Please keep it opened for the moment, I provided feedback to changes required in MainUI and also open a discussion about the OH icon server. I will later create separate issues in appropriate repos. |
I have now created the issue openhab/openhab-webui#1839 for issues in MainUI. |
Related to openhab/openhab-core#3052 Depends on the merge of PR openhab/openhab-core#3539 Signed-off-by: Laurent Garnier <[email protected]>
Related to openhab/openhab-core#3052 Depends on the merge of PR openhab/openhab-core#3539 Signed-off-by: Laurent Garnier <[email protected]>
Related to openhab/openhab-core#3052 Depends on the merge of PR openhab/openhab-core#3539 Signed-off-by: Laurent Garnier <[email protected]>
Related to openhab/openhab-core#3052 Depends on the merge of PR openhab/openhab-core#3539 Signed-off-by: Laurent Garnier <[email protected]>
Related to openhab/openhab-core#3052 Depends on the merge of PR openhab/openhab-core#3539 Signed-off-by: Laurent Garnier <[email protected]>
Related to openhab/openhab-core#3052 Depends on the merge of PR openhab/openhab-core#3539 Signed-off-by: Laurent Garnier <[email protected]>
Related to openhab/openhab-core#3052 Depends on the merge of PR openhab/openhab-core#3539 Signed-off-by: Laurent Garnier <[email protected]>
* New icon sources for an item Related to openhab/openhab-core#3052 Depends on the merge of PR openhab/openhab-core#3539 Signed-off-by: Laurent Garnier <[email protected]> * First set of review comments considered Signed-off-by: Laurent Garnier <[email protected]> * Remove the constraint regarding hyphen in icon name Signed-off-by: Laurent Garnier <[email protected]> * Consider one remaining opened comment Signed-off-by: Laurent Garnier <[email protected]> * Hide < and > Signed-off-by: Laurent Garnier <[email protected]> * Include HTTP link to icon sources directly in the table Signed-off-by: Laurent Garnier <[email protected]> --------- Signed-off-by: Laurent Garnier <[email protected]>
The category-field on an item allow to set an icon for the item. Historically this field would only allow the "oh"-icons but now also accepts other libraries by prefixing the icon name with the library name and a ":". This is a great thing.
The problem however is that this new interpretation isn't carried over to the "list item widget". When setting a "default list item widget" the entire content of the "category"-field is copied and prefixed bij "oh:". Even when the orginal field already contains a library name. This results in the fact that the icon isn't valid anymore.
A solution to this problem might be to check for an ":" in the category-field. The "oh:" should only be prefixed if no ":"-character can be found.
The text was updated successfully, but these errors were encountered: