-
-
Notifications
You must be signed in to change notification settings - Fork 267
Adds an UI command Item #2055
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
Adds an UI command Item #2055
Conversation
Job #1175: Bundle Size — 15.79MiB (+0.18%).Warning Bundle contains 16 duplicate packages - View duplicate packages Warning Bundle introduced 13 new packages: @jsep-plugin/regex, @jsep-plugin/arrow, @jsep-plugin/object and 10 more - View changed packages Bundle metrics (10 changes)
Bundle size by type (3 changes)
View job #1175 report View digitaldan:command-item branch activity |
|
Thanks @digitaldan, this seems like a useful feature. My main worry is the new SSE connection dedicated to it. As you might know (https://news.ycombinator.com/item?id=30313844) there's a 4-connections limit per domain (not per tab) on these so as the Main UI uses two for normal operation (one for state updates and one for relevant event updates dependent on the current page), then there's the webaudio one (#1422) and now this would potentially introduce another one. So that's up to 4 connections just for one tab of the Main UI, and in that case, if you open another tab (even if it's Basic UI or HABPanel) then it'll start to fail. Won't happen if you dedicate a tablet to displaying the UI but still. I would suggest trying to try to mitigate that by refactoring these two features (webaudio and UI commands) which rely on a permanent connection so they can share a single one (you can check which features are enabled in localCache and tweak the This could be a good opportunity to move all this logic to other file(s) (either imported or a mixin) dedicated to these permanent listeners as |
|
Update, I re-checked and apparently it's 6 concurrent EventSource connections, not 4 per domain like I always thought: https://bugs.chromium.org/p/chromium/issues/detail?id=275955
|
|
Yeah, the additional connection seems wasteful, i did not think about combining it with the audio sink one, thats a good idea until we can move to a websocket model. Agreed also to look at moving code outside of app.vue. |
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
9bacbde to
41a7793
Compare
|
I still need to test that audio continues to work, but this is ready to be looked at for code review. I also will need to add documentation about these new features. I updated my first post with all supported commands and more examples. |
|
Confirmed that the audio sink works as well |
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
florian-h05
left a comment
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, and works fine!
Really good idea, probably you got inspired by HABPanel's "Switch dashboard with item value"?
Please have a look at my comments.
Actually, i use https://github.com/vbier/habpanelviewer (well, a slightly modified version), but for our Main UI only, on a bunch of tablets around the house and use it's built in command item functionality extensively for reloading and navigating to pages on events, but this would replace most of that with the benefit of allowing me to use iPads if i need to ( my android tablets are very sluggish with our main UI). |
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
florian-h05
left a comment
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, just two minor comments.
I will test this ASAP and then merge it.
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
|
Thank @florian-h05 for the review ! |
|
Also, i probably need to write some documentation up on this, I guess adding this to https://github.com/openhab/openhab-docs/blob/main/ui/index.md would be the best path forward? |
|
Given that the title is "User Interface Design Overview", I would not expect the command Item there, however if the title is changed to "User Interface Overview", it could fit in there. If the docs are long, I would say introduce a new page. |
florian-h05
left a comment
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! I will merge this when the milestone build finished.
|
I think https://www.openhab.org/docs/settings/aboutpage.html would probably be a better location |
|
Yeah, there already is a section about Web Audio sink support. |
Addresses #2055 (comment). Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
…ing (#2157) Fixes #2153. Regression from #2055. MainUI was broken on older iOS versions, e.g. 15.8, because RegEx lookbehind assertions were introduced with Safari 16.4. This uses split by string instead and adds an ESLint rule to forbid the usage of RegEx lookbehind assertions. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
|
I noticed that the Audio Sink dosent work after a "Navigate to Page" Command UI is done after a short period of time, unless you manually click the new page. Its like the Audio Sink times out after a while when the Command UI is done (unless you click the page somewhere). This time-out dosent happen with the normal Navigate to Page or after you click somewhere on the screen "After" the Command UI navigated to a new page. Server Side: Client/Web Side: |
|
That's a limitation of modern browsers:
(https://next.openhab.org/docs/mainui/about.html#web-audio-sink) In case the command Item tells the UI to navigate to a different page, I guess the same happens. |
|
But it also does it with Edge |
|
I guess it is the same in all modern browsers. |
This allows the local running instance of the UI to listen to an item for commands. Specifically it will understand the following commands.
Commands with arguments use a colon delimiter, mirroring widget actions in the UI, for example
navigate:/pages/pagepopup:widget:customWidgetnotification:Text:Title:Sub Title:Title Right:5000I have this as a WIPas i play with this some more, but its very handy for tablets i have mounted around the house, i can easily pop up a camera, or a SIP call when the doorbell rings, trigger a reload when i have made UI changes or after a system update/restart, etc.....users can select the item in the about menu, since we have other local options set there its seemed to be the best place.