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

Items: Show non-semantic tags, relatesTo part of semantics & Use accordion tag input & Fix virtual box height #2087

Merged
merged 9 commits into from Sep 26, 2023

Conversation

Davek145
Copy link
Contributor

@Davek145 Davek145 commented Sep 23, 2023

Closes #2086

  • Adds Non-semantic tags to Items list (e.g. /settings/items/) consistently with similar list of Rules / Scripts / Pages.
  • Extends getItemTypeAndMetaLabel with relatesTo part of Semantic classification.
  • Fixes incorrect calculation of vue virtual box height on Items list.
  • Replaces custom tag input with accordion tag input (inspired by Page edit: Add editing of tags #2078) and show number of tags (inspired by Rule/script/scene edit: Update tag input & Refactoring #2083).
  • Moves the custom tag input to item-form.vue, so it is available also in Model view, when creating Item from Thing etc.

At minimum for the calculation of vue virtual box height it would be great to fully test this PR for variety of browsers. Currently I do not have complete BrowserStack access avaible, but anyway I have tried to test and adjust height calculation for following browsers using trial and my own devices (This PR is tested with BrowserStack):
Windows – Chrome, Edge, Firefox
MacOS – Chrome
Linux – Chrome, Firefox
iOS – Safari, Chrome
Android - Chrome
I kindly ask maintainers to test this PR using BrowserStack for other browsers. If I get full access from BrowserStack in the meantime, I will complete the testing with other browsers as well.

Semantic tags - display of relatesTo
Non-semantic tags - move to accordion

Signed-off-by: David Kesl <david_git@keslovi.cz>
Adds Non-semantic tags to Items list
Adds relatesTo in Semantic tag line
Fix of height in vue virtual list

Signed-off-by: David Kesl <david_git@keslovi.cz>
Signed-off-by: David Kesl <david_git@keslovi.cz>
@Davek145 Davek145 requested a review from a team as a code owner September 23, 2023 23:14
Signed-off-by: David Kesl <david_git@keslovi.cz>
Signed-off-by: David Kesl <david_git@keslovi.cz>
@Davek145
Copy link
Contributor Author

I now have full access to BrowserStack, so I have retested this PR once more for all combinations of most common browsers and devices.
Windows - Chrome, Edge, Firefox
Mac - Safari, Chrome, Firefox
Linux - Chrome, Firefox
Android - Chrome (phone/tablet)
iOS - Safari, Chrome (iPhone/iPad)
For all these browsers and devices functionality of PR and calculation of virtual box height is working correctly.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
…to Item mixin

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

@florian-h05 florian-h05 left a comment

Choose a reason for hiding this comment

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

Overall LGTM, thanks!

I have changed a few things, notably:

  • Rename the tag input in item-form.vue to Non-Semantic Tags to make clear that those are non-semantic. In this case IMO this is better, because Items support (in contrast to rules etc.) semantic tags.
  • Move the getItemTypeAndMetaLabel method to a mixin and therefore de-duplicate code. It is actually used at three places, you only updated two because you did not notice the third.
  • Use the isSemanticTag logic as used by tag-input.vue (move this to a mixin).
  • Move the getNonSemanticTags logic to a mixin, because it is used at several places in your PR.
  • Display tags in the item.vue component, so e.g. the list of group members looks the same as the Items list.

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

Thanks for fixing the vList height on so many platforms & browsers, my changes should not change anything in this behaviour, therefore I only tested this by switching between the different themes.

@florian-h05 florian-h05 added enhancement New feature or request main ui Main UI labels Sep 26, 2023
@florian-h05 florian-h05 added this to the 4.1 milestone Sep 26, 2023
@florian-h05 florian-h05 merged commit 05c3417 into openhab:main Sep 26, 2023
5 checks passed
@florian-h05 florian-h05 changed the title Items tag: added tags to list, unification of items tag edit Items: Show non-semantic tags, relatesTo part of semantics & Use accordion tag input Sep 26, 2023
@florian-h05 florian-h05 changed the title Items: Show non-semantic tags, relatesTo part of semantics & Use accordion tag input Items: Show non-semantic tags, relatesTo part of semantics & Use accordion tag input & Fix virtual box height Sep 26, 2023
@Davek145
Copy link
Contributor Author

Thanks a lot for cleansing the code. Using the mixin.js is good idea and removes the code duplication. Will take it as lesson learned for my next PR's.
Actually, I had similar idea and even tried to do it as well. Including using the existing isSemantic method. For some reason, it had huge performance hit on my large OH instance. That is why I have not used the call for semantic tags and instead used solely item metadata. Load time was multiple times faster.
However, when I try now your code, it works fast as well. There had to be something I have mised that did make that performance issue.

florian-h05 pushed a commit that referenced this pull request Sep 27, 2023
…2095)

Changes introduced into #2087 in combination with #2093 create duplicity
in showing non-semantic tags in the model page
(component/model/item-details.vue).
Non-semantic tags are displayed both in the Item and when edit mode is
enabled, in the accordion tag input in item-form.

Signed-off-by: David Kesl <david_git@keslovi.cz>
florian-h05 added a commit to florian-h05/openhab-webui that referenced this pull request Oct 14, 2023
Fixes openhab#2133.
Regression from openhab#2087.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
florian-h05 added a commit that referenced this pull request Oct 29, 2023
Fixes #2133.
Regression from #2087.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
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.

[MainUI] Tags in the Items list (/settings/items/)
2 participants