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

[basicui] Use command options for rendering Switch and Selection elements #217

Merged
merged 1 commit into from Apr 14, 2020

Conversation

cweitkamp
Copy link
Contributor

@cweitkamp cweitkamp commented Apr 14, 2020

  • Use command options for rendering Switch and Selection elements

Related to openhab/openhab-core#1016

grafik

Signed-off-by: Christoph Weitkamp github@christophweitkamp.de

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
@kaikreuzer
Copy link
Member

As this is in some way breaking existing sitemaps of users: Should we maybe keep that change for 3.0 and commit it on master only?

@cweitkamp
Copy link
Contributor Author

Yes, should be okay to merge it against master. I am not aware of any existing binding using command descriptions.

On the other hand I do not see a breaking change because the implementation of the getCommandDescription() method converts all existing StateOptions to CommandOptions in case no dedicated CommandDescription is provided.

https://github.com/openhab/openhab-core/blob/fb7a7ac42173d3228b7e4447582e827aa541e9ac/bundles/org.openhab.core/src/main/java/org/openhab/core/items/GenericItem.java#L415-L430

@kaikreuzer
Copy link
Member

On the other hand I do not see a breaking change

Ok, fair point, so merging it here is ok for me.
But still: It'll be more important on master, so I hope you will port it and provide another PR there :-)

@kaikreuzer kaikreuzer merged commit e19d308 into openhab:2.5.x Apr 14, 2020
@cweitkamp cweitkamp deleted the feature-1016-command-options branch April 14, 2020 20:28
cweitkamp added a commit to cweitkamp/openhab-webui that referenced this pull request Apr 14, 2020
…hab#217)

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
cweitkamp added a commit to cweitkamp/openhab-webui that referenced this pull request Apr 14, 2020
…hab#217)

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
@cweitkamp
Copy link
Contributor Author

@kaikreuzer Done (see #220). Easy going - thanks to cherry-pick.

@kaikreuzer
Copy link
Member

That was quick, thanks :-)
Did the cherry picking work even though the namespace of classes has changed? I guess you needed to help this a bit to resolve!

kaikreuzer pushed a commit that referenced this pull request Apr 14, 2020
#220)

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
@cweitkamp
Copy link
Contributor Author

cweitkamp commented Apr 14, 2020

It worked in general. Yes, I had to solve merge conflicts due to namespace change. But only for the imports - which was not a big deal.

@lolodomo
Copy link
Contributor

I think that the default should remain a selection. The buttons are not adapted when you have 50 options !

@cweitkamp
Copy link
Contributor Author

The decision for the default element is done in OHC (openhab/openhab-core#1422).

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.

None yet

3 participants