-
-
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
[Sitemap] Add new color keyword "itemValue" #3453
Conversation
What item color is being used when the color is with a condition? Would it pick the color of the item being shown, or the color of the item on which the condition applies? |
the color of the item being shown |
...rest.sitemap/src/main/java/org/openhab/core/io/rest/sitemap/internal/PageChangeListener.java
Outdated
Show resolved
Hide resolved
Depends on openhab/openhab-core#3453 Signed-off-by: Laurent Garnier <lg.hc@free.fr>
Depends on openhab/openhab-core#3453 Signed-off-by: Laurent Garnier <lg.hc@free.fr>
832f56b
to
d3e4b89
Compare
openhab/openhab-core#3453 Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, otherwise LGTM.
if (item instanceof ColorItem && item.getState() instanceof HSBType) { | ||
HSBType hsbState = (HSBType) item.getState(); | ||
itemRGBHexCode = "#" + Integer.toHexString(hsbState.getRGB()).substring(2); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (item instanceof ColorItem && item.getState() instanceof HSBType) { | |
HSBType hsbState = (HSBType) item.getState(); | |
itemRGBHexCode = "#" + Integer.toHexString(hsbState.getRGB()).substring(2); | |
} | |
if (item.getState() instanceof HSBType hsbState) { | |
itemRGBHexCode = "#" + Integer.toHexString(hsbState.getRGB()).substring(2); | |
} |
If the state is of HSBType
it must be a ColorItem
. You can further reduce code by using pattern matching.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your proposal will lead to a NPE when item is null.
But I could have a State as parameter rather than an Item if we consider that a test on HSBType is sufficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
item != null
would then be sufficient. But does it make sense to call the method at all if the item is null? If you need to add a check for each call, then I would prefer to add item != null
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed. I am now passing as argument the item state.
Closes openhab#3429 Signed-off-by: Laurent Garnier <lg.hc@free.fr>
d3e4b89
to
15760ac
Compare
Depends on openhab/openhab-core#3453 Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@J-N-K : gentle ping |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM,thanks,
Depends on openhab/openhab-core#3453 Signed-off-by: Laurent Garnier <lg.hc@free.fr>
* [Sitemap] Add color 'itemValue' openhab/openhab-core#3453 Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com> * update Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com> --------- Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>
@lolodomo - should this be mentioned in the documentation? |
Doc has been updated. |
Closes openhab#3429 Signed-off-by: Laurent Garnier <lg.hc@free.fr> GitOrigin-RevId: 52e36a0
Closes #3429
Signed-off-by: Laurent Garnier lg.hc@free.fr