Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

[Classic Icon Set] "presence-on" icon is missing #5416

Closed
Confectrician opened this issue Apr 14, 2018 · 8 comments · Fixed by #5566
Closed

[Classic Icon Set] "presence-on" icon is missing #5416

Confectrician opened this issue Apr 14, 2018 · 8 comments · Fixed by #5566

Comments

@Confectrician
Copy link
Contributor

Hi,

This is a port from openhab/openhab-docs#660

It seems that somethings going wrong in https://github.com/eclipse/smarthome/blob/master/extensions/ui/iconset/org.eclipse.smarthome.ui.iconset.classic/README.sh and the html tags for the presence icon are generated wrong.

@sjsf
Copy link
Contributor

sjsf commented Apr 18, 2018

@ItIsFelix the mentioned script at least "works as designed". What would have been your expectation wrt to the alt property in case of the presence icon?

triller-telekom added a commit to triller-telekom/smarthome that referenced this issue Apr 18, 2018
Fixes eclipse-archived#5416

Signed-off-by: Stefan Triller <stefan.triller@telekom.de>
@ItIsFelix
Copy link

ItIsFelix commented Apr 18, 2018

Hallo sjka, for the icon presence.png I see the alt-property "presence-off".
That looks strange to me.
I would expect that the alt-property is "presence".
See here: OpenHab - Classic Icon Set

Thanks
Felix

triller-telekom added a commit to triller-telekom/smarthome that referenced this issue Apr 18, 2018
Fixes eclipse-archived#5416

Signed-off-by: Stefan Triller <stefan.triller@telekom.de>
@triller-telekom
Copy link
Contributor

@ItIsFelix: I have fixed that in the linked PR. Thank you for reporting it!

@sjsf
Copy link
Contributor

sjsf commented Apr 18, 2018

It generally list all of the available state variants (if any). Have a look here at e.g. battery, blinds, door, frontdoor, garagedoor,...

So, let me ask again: what did you expect to see there? I.e.

  • Do you think it is surprising that the alt="..." property contains all the available state variants and you would have expected it to contain only the category name? This would however apply to all of the icons, not only to the presence in particular.
  • Or are you rather missing a presence-on variant of the presence icon in particular? This indeed doesn't currently exist - that's why I mentioned that the script "works as designed".

@ItIsFelix
Copy link

ItIsFelix commented Apr 18, 2018

Hello @SJKA,
you are right. An alt-property like "presence" is not correct.
I was expecting an alt-property "presence-off presence-on"

I didn't realize that there is no presence-on.png

@sjsf sjsf changed the title [Documentation]Wrong alt property for presence.png [Classic Icon Set] "presence-off" icon is missing Apr 18, 2018
@ItIsFelix
Copy link

@SJKA Sorry for confusing you. The icon "presence-on.png" is missing.
Not the icon "presence-off.png"
Can you please change the title.

@sjsf sjsf changed the title [Classic Icon Set] "presence-off" icon is missing [Classic Icon Set] "presence-on" icon is missing Apr 18, 2018
@triller-telekom
Copy link
Contributor

@ItIsFelix the "alt" property of an "img" tag is supposed to express what the image is about for text only browsers or browsers helping blind people. So it should show what the title below says.

Also in the "Places" section of the documentation this is correct for every icon already.

@ItIsFelix
Copy link

In theory I agree with you.
The alt attribute specifies an alternate text for an image, if the image cannot be displayed or if a user for some reason cannot view it.
But in this case, the alt attribute is used to show the different states of icons. (see the fire icon or switch icon)

Now we can say there is a icon named "presence" to display the presence (at home) and a icon named "presence-off" to display the absense.
A third icon is not needed because there are only two valid states.

Or we can do it like all other "switch" icons.
There is a base Item like the icon named "presence" to display a base icon (maybe it should look like the group icon), a "presence-on" to display the presence and a "presence-off" to display the absense.

sjsf added a commit to sjsf/smarthome that referenced this issue May 11, 2018
fixes eclipse-archived#5416
Signed-off-by: Simon Kaufmann <simon.kfm@googlemail.com>
kaikreuzer pushed a commit that referenced this issue May 14, 2018
* add missing presence-on icon

fixes #5416
Signed-off-by: Simon Kaufmann <simon.kfm@googlemail.com>
ermartens pushed a commit to ermartens/smarthome that referenced this issue Jun 15, 2018
* add missing presence-on icon

fixes eclipse-archived#5416
Signed-off-by: Simon Kaufmann <simon.kfm@googlemail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants