-
-
Notifications
You must be signed in to change notification settings - Fork 243
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
[blockly] Add new sophisticated notification blocks #2672
[blockly] Add new sophisticated notification blocks #2672
Conversation
Signed-off-by: Stefan Höhn <mail@stefanhoehn.com>
Signed-off-by: Stefan Höhn <mail@stefanhoehn.com>
#2138 Bundle Size — 10.77MiB (+0.91%).Warning Bundle contains 2 duplicate packages – View duplicate packages Bundle metrics
|
Current #2138 |
Baseline #2134 |
|
---|---|---|
Initial JS | 1.88MiB |
1.88MiB |
Initial CSS | 607.91KiB |
607.91KiB |
Cache Invalidation | 18.22% |
17.96% |
Chunks | 223 |
223 |
Assets | 246 |
246 |
Modules | 2888 |
2888 |
Duplicate Modules | 149 |
149 |
Duplicate Code | 1.84% (-0.54% ) |
1.85% |
Packages | 97 |
97 |
Duplicate Packages | 2 |
2 |
Bundle size by type 1 change
1 regression
Current #2138 |
Baseline #2134 |
|
---|---|---|
JS | 8.96MiB (+1.1% ) |
8.86MiB |
CSS | 892.47KiB |
892.47KiB |
Fonts | 526.1KiB |
526.1KiB |
Media | 295.6KiB |
295.6KiB |
IMG | 140.74KiB |
140.74KiB |
HTML | 1.24KiB |
1.24KiB |
Other | 871B |
871B |
Bundle analysis report Branch stefan-hoehn:blockly_notificatio... Project dashboard
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
The action button syntax seems a little cumbersome. I assume splitting label and action there is not possible? |
Kinda correct. I thought about it The problem is that it would require a special designed "command-button" input block that contains two inputs by itself. It would require a special "command block" to be implemented and a very special handling in that case. It could maybe be done but surely is a much more complex implementation while I don't think it is actually not too bad the way it is... |
And one more comment, @maniac103
So stay tuned for an update. |
I assume you mean 'and not command', right? But I indeed think that's much better, but...
...is beaten by this approach which sounds awesome 👍 |
Signed-off-by: Stefan Höhn <mail@stefanhoehn.com>
Signed-off-by: Stefan Höhn <mail@stefanhoehn.com>
Signed-off-by: Stefan Höhn <mail@stefanhoehn.com>
Signed-off-by: Stefan Höhn <mail@stefanhoehn.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.
Overall already looks good, please have a look at my comments.
I also have a few wording remarks:
Replace "with Action" -> "with On-Click Action" and rename the action button block's "on-click action" -> "action".
Are shadow blocks inside me screenshot expected to work? I think in blockly-editor.vue are a few input names wrongly defined.
bundles/org.openhab.ui/web/src/assets/definitions/blockly/blocks-notifications.js
Outdated
Show resolved
Hide resolved
bundles/org.openhab.ui/web/src/assets/definitions/blockly/blocks-notifications.js
Outdated
Show resolved
Hide resolved
bundles/org.openhab.ui/web/src/assets/definitions/blockly/blocks-notifications.js
Outdated
Show resolved
Hide resolved
bundles/org.openhab.ui/web/src/assets/definitions/blockly/blocks-notifications.js
Show resolved
Hide resolved
bundles/org.openhab.ui/web/src/assets/definitions/blockly/blocks-notifications.js
Show resolved
Hide resolved
bundles/org.openhab.ui/web/src/assets/definitions/blockly/blocks-notifications.js
Show resolved
Hide resolved
bundles/org.openhab.ui/web/src/assets/definitions/blockly/blocks-notifications.js
Outdated
Show resolved
Hide resolved
bundles/org.openhab.ui/web/src/components/config/controls/blockly-editor.vue
Outdated
Show resolved
Hide resolved
bundles/org.openhab.ui/web/src/components/config/controls/blockly-editor.vue
Show resolved
Hide resolved
bundles/org.openhab.ui/web/src/assets/definitions/blockly/blocks-notifications.js
Outdated
Show resolved
Hide resolved
Signed-off-by: Stefan Höhn <mail@stefanhoehn.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.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.
LGTM now, I have just pushed a few minor changes - thanks for your great work!
Unfortunately I found a few bugs and I am already working on it. The fix will come very soon. |
There was an error on code generation in particular when using vars and string concat blocks (who sometime generate code instead of pure strings) which is fixed now. I also gave notifications its individual color to distinguish them better from other groups. Follow-up for #2672. --------- Signed-off-by: Stefan Höhn <mail@stefanhoehn.com>
This is probably the most comprehensive block I have provided so far which supports the awesome cloud notification functionality that was provided recently. Kudos to all who have provided the cloud notifications and added them to the apps! ❤️
Thanks for the support during this implementation to Florian, Dan, Danny, Matthias (and the rest of the openHAB foundation gang who provided feedback and nice ideas to make it even better).
I tested all of the above use cases and I hope I covered most of it.
Note that the new blocks can completely replace the old ones (though they will not be automatically converted but it must be done manually). I will document this eventually in the blockly docs.
The new notification blocks:
The block can be highly customized by showing or hiding the required inputs:
Without user ids, it will result into a broadcast. Individual users can be address with a comma separated list:
You can provide actions (directly or further down below via up to three clickable buttons in the notification)
Two different types of actions are available to attach to direct action or a button action
Action:
Button 1/2/3 Action
Action Blocks
Actions can be provided via blocks are generically via Strings:
Many more examples:
Based on
https://www.openhab.org/addons/integrations/openhabcloud/#cloud-notification-actions
https://www.openhab.org/addons/automation/jsscripting/#cloud-notification-actions
Here is an example in the Android app with included media: