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

Dynamically load the list of Semantic tags and store them in Vuex #1882

Merged
merged 7 commits into from Jun 28, 2023

Conversation

florian-h05
Copy link
Contributor

@florian-h05 florian-h05 commented May 8, 2023

Supersedes #1850.
Closes #1822.

Depends on openhab/openhab-core#3559 (already merged now).
Adding custom semantic tags is now possible: openhab/openhab-core#3519.

This PR loads the Semantic tags when MainUI is loaded the first time and stores them in Vuex.
This allows the removal of the hard-coded Semantic tags and the translations from the assets and therefore makes the initial JS smaller.

@relativeci
Copy link

relativeci bot commented May 8, 2023

Job #1039: Bundle Size — 15.68MiB (-0.6%).

2cd37b9(current) vs 511d887 main#1038(baseline)

⚠️ Bundle contains 16 duplicate packages

Metrics (5 changes)
                 Current
Job #1039
     Baseline
Job #1038
Initial JS 1.66MiB(-4.05%) 1.74MiB
Initial CSS 608.89KiB 608.89KiB
Cache Invalidation 93.95% 93.92%
Chunks 219 219
Assets 689 689
Modules 1692(-1.63%) 1720
Duplicate Modules 87(+2.35%) 85
Duplicate Code 1.76%(+2.33%) 1.72%
Packages 138 138
Duplicate Packages 15 15
Total size by type (2 changes)
                 Current
Job #1039
     Baseline
Job #1038
CSS 858.62KiB 858.62KiB
Fonts 526.1KiB 526.1KiB
HTML 1.23KiB 1.23KiB
IMG 140.74KiB 140.74KiB
JS 9.2MiB (-0.67%) 9.26MiB
Media 295.6KiB 295.6KiB
Other 4.7MiB (-0.69%) 4.74MiB

View job #1039 reportView main branch activity

@florian-h05
Copy link
Contributor Author

florian-h05 commented May 8, 2023

There seems to be an issue that causes the translations not to be loaded, because the Language header is missing. I'll check how the rest of the UI loads correct translations and how to do it here.
Ooops, I had the wrong language set up in my browser.

Anyway, there was a bug with localisation of the dynamically loaded tags that I've just fixed.

@florian-h05 florian-h05 added rebuild trigger a new Jenkins job and removed rebuild trigger a new Jenkins job labels May 8, 2023
@florian-h05 florian-h05 marked this pull request as ready for review May 8, 2023 16:44
@florian-h05 florian-h05 requested a review from a team as a code owner May 8, 2023 16:44
@florian-h05 florian-h05 added enhancement New feature or request main ui Main UI labels May 8, 2023
@florian-h05 florian-h05 added this to the 4.0 milestone May 8, 2023
@ghys
Copy link
Member

ghys commented May 11, 2023

and therefore makes the initial JS smaller.

Yes but it's usually put in cache whereas the additional REST query might not be :)

I see a lot of translations removed in this PR but I would like to have them transferred somehow to their new place, probably in openhab-core, so they are not lost.

@florian-h05
Copy link
Contributor Author

whereas the additional REST query might not be :)

Yeah, but to put them in cache we’d need caching support in core (last-modified etc)

I see a lot of translations removed in this PR but I would like to have them transferred somehow to their new place

There are already translations in openHAB core (see https://github.com/openhab/openhab-core/tree/main/bundles/org.openhab.core.semantics/src/main/resources), however I’m not sure if all translations from webui are there.

@ghys
Copy link
Member

ghys commented May 11, 2023

Yes these are the synonyms for a tag (as you can see there could be more than one), and they could be complemented by the "synonyms" metadata.
The semantic-tag-to-localized-card-label is a different matter.

@florian-h05
Copy link
Contributor Author

The semantic-tag-to-localized-card-label is a different matter.

Core seems to take the translations from those synonyms files, so when removing our translations here we should make sure that all of them are included in the synonyms translations.

@florian-h05
Copy link
Contributor Author

florian-h05 commented May 14, 2023

These translations are (partly) missing at openHAB Core:

  • Bg Bularian
  • Ca Catalan
  • Fa Farsi (Persian)
  • Lb Luxembourgish
  • Lv Latvian
  • Sk Slovak
  • Sv Swedish

@ghys IMO the problem with migrating them is, that these translations are all only for singular forms of the semantics and without synonyms, but openHAB Core wants singular, plural and synonyms. Obviously, I cannot translate the missing parts.

I propose to merge this PR and I will contact the contributors of these 7 listed translations that will be lost and ask for translating the semantics in openHAB Core. For Luxembourgish, you can probably translate?

@kaikreuzer @cweitkamp
I wanted to check translations for core in CrowdIn, however I cannot finde these languages in the openhab-core project: Persian, Latvian, Slovak. I guess they are disabled, so can you please enable them?

@florian-h05
Copy link
Contributor Author

@ghys Please see my comment. IMO we cannot block (UI) progress because of a few translations.

@lolodomo
Copy link
Contributor

@florian-h05 : openhab/openhab-core#3646 has been merged.
Can you please check that it does not impact your PR?

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>
Refs openhab/openhab-core#3646.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05
Copy link
Contributor Author

@lolodomo It required adjustments, your core PR was breaking.

@ghys This is ready for review and AFAIK the REST API for tags is finished, so we should not need any more updates/adjustments here. Can you please review and merge?

@lolodomo
Copy link
Contributor

loads the Semantic tags when MainUI is loaded the first time

What do you mean by the first time?
Isn't it each time MainUI is opened in a browser?
As it is now possible to update the list of tags, this loading should happen at least at each MainUI startup.

This should make it much easier to trigger a reload from other parts of
the UI once this is needed in the future.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05
Copy link
Contributor Author

AFAIK the loadData method in app.vue is only called when MainUI loads or some events are emitted, so everything there (e.g. pages, sitemaps etc.) is not automatically refreshed from the server as long as you don‘t reload the browser tab or reload MainUI from the „Help & About“ page.

To automatically make those refresh in MainUI, they need to be loaded every time. We could use ETags or Last-Modified do cache responses to avoid unnecessary traffic, but I‘d rather do this in another PR and it probably also requires core changes. There already is a PR to use this for Items, Things and rules, see #1661.

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.

For Luxembourgish, you can probably translate?

Ech kann e bisschen lëtzebuergesch schwätzen... but it's not my forte...! (My German is better actually).

I'm good with the changes in the end, let's get it to OH4!

@ghys ghys merged commit 46cf029 into openhab:main Jun 28, 2023
5 checks passed
@florian-h05 florian-h05 deleted the semantics-load branch June 28, 2023 21:10
@lolodomo
Copy link
Contributor

I will test the next snapshot to see how it behaves with user semantic tags.

@lolodomo
Copy link
Contributor

lolodomo commented Jul 1, 2023

@florian-h05 : it looks like it works well. I added (using REST API) two new locations and 1 new equipment. After restarting MainUI, I can see these new semantic tags when I create a new group in Main UI and finally the new locations and the new kind of equipment appear in Location and Equipment tabs. Very good.

Do you think you will find the time before OH4 is released to add a new page to manage semantic tags in Main UI ?

@ghys
Copy link
Member

ghys commented Jul 2, 2023

A quick reminder that cache control should be implemented in core for the GET /rest/tags endpoint like in openhab/openhab-core#3335 as the list is requested before the UI is even displayed. Caching the tags makes a lot of sense as it doesn't change often (or at all), to avoid about 17kb of avoidable traffic and a few milliseconds of delay. Every bit helps when using the UI remotely (OH Cloud or other).

See
openhab/openhab-core@6e83d3f#diff-4656a36e72716d544cfc5692a3e5409cd59b7f26ae9f8d97af4a9e18c43adc9a
for an example.
No staticDataOnly parameter is even necessary in this case as all data is static. This means no change should be needed in the UI code.

@florian-h05
Copy link
Contributor Author

florian-h05 commented Jul 2, 2023

it looks like it works well.

👍

Do you think you will find the time before OH4 is released to add a new page to manage semantic tags in Main UI ?

Unfortunately I’m very busy this week and away next week (10th to 14th July), so I fear that I won’t make it before the feature freeze. I’m hoping to at least finish the UoM stuff for the new release.

A quick reminder that cache control should be implemented in core for the GET /rest/tags endpoint like in openhab/openhab-core#3335 as the list is requested before the UI is even displayed.

Thanks for noticing that. @lolodomo Can you do that?

@florian-h05
Copy link
Contributor Author

FYI: I have contacted the CrowdIn contributors a few seconds ago and asked them to re-contribute their translations at openhab-core.

@kaikreuzer I also noted that the removed translations from MainUI are still in CrowdIn (https://crowdin.com/translate/openhab-webui/384), can you please remove them?

@kaikreuzer
Copy link
Member

@florian-h05 Could you tell me what exactly you want me to remove?

@florian-h05
Copy link
Contributor Author

florian-h05 commented Jul 2, 2023

With this PR, MainUI loads the semantic tags together with the translations from the REST API and we therefore do not use the translations at openhab-webui anymore. This PR also removed the transation files from the repo, but seems like this was not synced back to CrowdIn.

@kaikreuzer EDIT: The translations to remove are semantics/en.json from the org.openhab.ui bundle.

ghys pushed a commit that referenced this pull request Jul 26, 2023
Fixes #1976.
Follow-up for #1882.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
florian-h05 added a commit that referenced this pull request Jul 26, 2023
Fixes #1976.
Follow-up for #1882.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
(cherry picked from commit 2230f52)
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.

Get the list of semantic tags from REST
4 participants