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

Add tags & script language to script settings #1601

Merged
merged 7 commits into from
Jan 3, 2023

Conversation

florian-h05
Copy link
Contributor

@florian-h05 florian-h05 commented Dec 22, 2022

Reference #1597.
Reference openhab/openhab-addons#14005.

Description

This PR extends the script settings to allow the user to set tags and change the MIME type after creation of the script.
The ability to add and edit tags was requested several times (I think also by @rkoshak).

The ability to change the MIME type of a script after creation is very important for openHAB 4, because Nashorn will (very likely) swap it's MIME type with GraalJS, but Nashorn scripts do not run on GraalJS without some changes. With the extended script settings, the user can just change the MIME type to the new MIME type of Nashorn.

The "Script" tag that all scripts have is hidden in the settings.

How it looks

Script Creation

image

Note that the "Tags" section is newly added and doesn't show the "Script" tag although it is present.

Editing a Script

Here has already been a settings popup. It has been extended with tags and the ability to change the MIME type:
image

Note that this script settings are from a Blockly script, therefore only Javascript versions (I've only installed GraalJS) are listed. If you open these settings for a normal script, you can change the MIME type to any of the available engines, e.g. Rules DSL.

Editing a Script in a Rule

The settings are newly introduced for this case and limited to the MIME type:
image

/ping @ghys @rkoshak @stefan-hoehn

@florian-h05 florian-h05 requested a review from a team as a code owner December 22, 2022 18:16
@relativeci
Copy link

relativeci bot commented Dec 22, 2022

Job #667: Bundle Size — 15.96MiB (+0.01%).

2098fa1(current) vs 0f52ce4 main#666(baseline)

Metrics (1 change)
                 Current
Job #667
     Baseline
Job #666
Initial JS 1.73MiB 1.73MiB
Initial CSS 608.52KiB 608.52KiB
Cache Invalidation 90.47% 90.46%
Chunks 218 218
Assets 688 688
Modules 2007 2007
Duplicate Modules 108 108
Duplicate Code 1.8% 1.8%
Packages 133 133
Duplicate Packages 15 15
Total size by type (2 changes)
                 Current
Job #667
     Baseline
Job #666
CSS 856.56KiB 856.56KiB
Fonts 1.08MiB 1.08MiB
HTML 1.23KiB 1.23KiB
IMG 140.74KiB 140.74KiB
JS 9.04MiB (+0.02%) 9.03MiB
Media 295.6KiB 295.6KiB
Other 4.59MiB (+0.02%) 4.59MiB

View job #667 reportView main branch activity

@stefan-hoehn
Copy link
Contributor

I love the idea. Again a step forward to easier allow to the new ECMAScript version.

regarding the tags:

I recently added some more search criteria in /settings/rules/ (see #1540)
Maybe it is possible to show and search for the tags there (like in /settings/items/) as well?

@florian-h05
Copy link
Contributor Author

I've just checked it:

  • searching by tags works
  • searching by script states works
  • searching by UID works
  • searching by description works

FYI: The code to list rules and the one to list scripts is the same and scripts are in fact only a special type of rules.

@florian-h05
Copy link
Contributor Author

@ghys This is now ready for review, it's working fine on my system and doesn't print any errors to the console.

@stefan-hoehn
Copy link
Contributor

Does it not only search for the tags but also show the tags?. Our idea until now was that everything that is searchable is also shown on the list and vice versa.

@florian-h05
Copy link
Contributor Author

It also shows the tags -- it behaves exactly the same like the rule overview.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Copy link
Member

@ghys ghys left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

Another idea I had in to add a blue "Edit YAML" list button that would let you edit the script module resp. the entire script rule in a popup editor (similar to what you have in chart pages when you add e.g. a tooltip), but that's already very nice to have and can always be done later.

@@ -243,6 +231,22 @@ export default {
this.load()
})
},
loadScriptModules () {
this.$oh.api.get('/rest/module-types/script.ScriptAction').then((data) => {
Copy link
Member

Choose a reason for hiding this comment

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

One tiny general remark:
this component is also used to edit script conditions.
I understand you probably took this into account and figured there was no reason the list of languages would be different from the action to the condition, so good enough, but just pointing it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for that information.
I wasn‘t aware of this, but as there is no difference in the list of available languages for actions and conditions, this should be fine.
If I see it correctly, this component also did not take into account whether it edits an action or a condition before this PR.

@ghys ghys merged commit 89e82f4 into openhab:main Jan 3, 2023
@ghys ghys added enhancement New feature or request main ui Main UI labels Jan 3, 2023
@ghys ghys added this to the 4.0 milestone Jan 3, 2023
@ghys ghys changed the title [MainUI] Add tags & script language to script settings Add tags & script language to script settings Jan 3, 2023
@florian-h05 florian-h05 deleted the script-extend-settings branch January 3, 2023 14:28
crnjan pushed a commit to crnjan/openhab-webui that referenced this pull request Jan 17, 2023
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Boris Krivonog <boris.krivonog@inova.si>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request main ui Main UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants