Skip to content
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

Icons shall not be changed in rules #2222

Merged
merged 2 commits into from
Feb 26, 2024

Conversation

dilyanpalauzov
Copy link
Contributor

• insert the conclusion from openhab/openhab-core#3958

Copy link

netlify bot commented Jan 31, 2024

Thanks for your pull request to the openHAB documentation! The result can be previewed at the URL below (this comment and the preview will be updated if you add more commits).

Name Link
🔨 Latest commit 992368c
🔍 Latest deploy log https://app.netlify.com/sites/openhab-docs-preview/deploys/65dc671cb9644d0008f4f12b
😎 Deploy Preview https://deploy-preview-2222--openhab-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@florian-h05 florian-h05 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this information should be added to the Items config page, possibly somewhere around there: https://github.com/openhab/openhab-docs/blob/main/configuration/items.md#icons.

The categories concepts page is outdated and not promoted on the docs website and IMO also does not add any information that is not found somewhere else, so IMHO it should be removed.

@dilyanpalauzov
Copy link
Contributor Author

I have deleted concepts/categories.md and added Icons (categories) should not be changed dynamically via Rules. to configuration/items.md.

@dilyanpalauzov dilyanpalauzov force-pushed the categories_sidebar branch 2 times, most recently from 8401c3e to ea3ecea Compare February 24, 2024 12:55
@dilyanpalauzov dilyanpalauzov changed the title Categories: include in sidebar Icons shall not be changed in rules Feb 24, 2024
@@ -322,6 +322,8 @@ Note that image files with the wrong file ending will be ignored.

Users may substitute their own icon for an icon from the default icon set by placing a file in the `$OPENHAB_CONF/icons/classic/` folder with the same filename as the name of the icon being substituted.

Icons (categories) should not be changed dynamically via Rules.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather put this in after line 299 and rephrase it a bit:

Please note that icons (also know as categories) are not meant to be changed dynamically via rules.


## Thing Categories

The Thing type definition allows to specify a category.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have noticed that this part of the page is actually referenced by the developer Thing XML docs:
https://github.com/search?q=repo%3Aopenhab%2Fopenhab-docs%20categories.html&type=code

I am not sure how to process, but AFAIK the available categories are basically the list of icons with the icon names in the typical Java class naming style (e.g. FrontDoor instead of lowercase icon name frontdoor).
The category of a Thing and channels is AFAIK only setting the icon and not doing anything else.

Therefore I propose to remove the references to this page, and instead write under the Thing categories and the channel categories heading something like:

Thing [channel] categories are used to set an icon for that Thing [channel]. The available categories correspont with the available icons of the classic icon set, however categories are written in Java class-naming style, e.g. FrontDoor instead of lowercase frontdoor.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see how can Channels or Things get an icon/a category in file-based definitions. Is it possible to set categories for Things and Channels only at development time, and these are then fixed forerer in the .jar file?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly, these are defined by the add-on developer inside the thing-type and channel-type XML definitions and used by the UI as suggested icons when you „create Items from equipment“.

@dilyanpalauzov
Copy link
Contributor Author

The last changes represent my understanding of the comment above.

Copy link
Contributor

@florian-h05 florian-h05 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good, I‘ve added a few comments.
Please do not force push from now on, but simply add a new commit.

@@ -68,6 +68,7 @@ module.exports = [
'concepts/discovery',
'concepts/audio',
'concepts/units-of-measurement',
'concepts/profiles',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please remove this change? Let‘s focus on the icons in this PR.

@@ -58,7 +58,7 @@ In that way, a generic thing type could be listed for users and a corresponding

### Thing Categories

A description about thing categories as well as an overview about which categories exist can be found in our [categories overview](../../concepts/categories.html).
Thing categories are used to set an icon for that Thing. The available categories correspond with the available icons of the classic icon set, however categories are written in Java class-naming style, e.g. FrontDoor instead of lowercase frontdoor.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Thing categories are used to set an icon for that Thing. The available categories correspond with the available icons of the classic icon set, however categories are written in Java class-naming style, e.g. FrontDoor instead of lowercase frontdoor.
Categories are used to provide meta information about Things. Thing categories describe how the physical device **looks like**. UIs can use this information e.g. to render icons.
The available categories correspond with the [available icons of the classic iconset]({{base}}/configuration/iconsets/classic/), however categories are written in Java class-naming style, e.g. `FrontDoor` instead of lowercase `frontdoor`.

@@ -411,7 +408,7 @@ public class ExampleHandlerFactory extends BaseThingHandlerFactory {

### Channel Categories

A description about channel categories as well as an overview about which categories exist can be found in out [categories overview](../../concepts/categories.html).
Channel categories are used to set an icon for that Channel. The available categories correspond with the available icons of the classic icon set, however categories are written in Java class-naming style, e.g. FrontDoor instead of lowercase frontdoor.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Channel categories are used to set an icon for that Channel. The available categories correspond with the available icons of the classic icon set, however categories are written in Java class-naming style, e.g. FrontDoor instead of lowercase frontdoor.
Channel categories are used to provide meta information about channels. Channel categories describe the **functional purpose** of the channel and are used by the UI to render icons.
The available categories correspond with the [available icons of the classic iconset]({{base}}/configuration/iconsets/classic/), however categories are written in Java class-naming style, e.g. `BatteryLevel` instead of lowercase `batterylevel`.

@@ -434,7 +431,7 @@ Inside the thing types XML file channel groups can be defined like this:
```

The channel group type is defined on the same level as the thing types and channel types.
The group type must have a label, an optional description, and an optional [category](../../concepts/categories.html).
The group type must have a label, an optional description, and an optional category (icon).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The group type must have a label, an optional description, and an optional category (icon).
The group type must have a label, an optional description, and an optional category (e.g. used to render an icon).

Copy link
Contributor

@florian-h05 florian-h05 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now.

Haven’t I clearly stated that you should NOT force push from my last comment on?
This is the last time I will review a PR from you with force pushes in the review process.

@stefan-hoehn stefan-hoehn merged commit 1a96bd8 into openhab:main Feb 26, 2024
5 checks passed
@dilyanpalauzov dilyanpalauzov deleted the categories_sidebar branch February 26, 2024 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants