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

Labelling of items in Model tab pages (Equipment & Properties) #1264

Merged
merged 3 commits into from
Jan 16, 2022

Conversation

tarag
Copy link
Contributor

@tarag tarag commented Jan 6, 2022

In order to have something understandable in Equipment/Properties model tab pages when having many Equipment and/or Properties of the same type (potentially having the same user-facing label), it is necessary to label them with something that allows to distinguish them from one another, which is usually the location of the equipment/point item.

This labelling is redundant with the labelling of the Location items to which these Equipment/Properties items may belong and leads to additional configuration work. In addition, it will clutter the pages in the Locations tab because all the items displayed there would have one common part which is the location, providing no user value in this context but more visual load (particularly problematic on small mobile screen where the end of the label, which may contain the most meaningful part will be trimmed).

This pull request provides some changes providing model context for the Equipment and Properties model tab pages. This should allow having simple labels definition for items belonging to the model:

  • In Equipment tab page cards: display the Location / Upper-level-equipment path in the divider title before items belonging to some equipment, trimming to preserve the end of this label if needed
  • In Properties tab page cards: display the Location / Upper-level-equipment path in the subtitle of the item, instead of the item name (which I believe is not meant to be displayed in first-level user-facing non admin-level task pages)

Here is an illustration of the issue with the current version (3.2.0) with either short or complete labels used, and the resulting effect of the patch using the short labels.

Current status with model-redundant labels
current-long labels

Current status with short labels
current-short labels

Pull request version with short labels
patched-short labels

For each Item part of the model, its parent path in model is parsed from API response and relations added to the item. Allows path label to be displayed in Equipment and Properties pages.

Signed-off-by: Gautier Taravella <tarag@mailbox.org>
…ts used as dividers.

Signed-off-by: Gautier Taravella <tarag@mailbox.org>
@tarag tarag requested a review from a team as a code owner January 6, 2022 14:02
@relativeci
Copy link

relativeci bot commented Jan 6, 2022

Job #329: Bundle Size — 10.67MB (+0.02%).

e230fc0 vs df671b5

Changed metrics (4/8)
Metric Current Baseline
Initial JS 1.67MB(+0.16%) 1.66MB
Initial CSS 605.01KB(~+0.01%) 604.97KB
Cache Invalidation 25.35% 3.33%
Modules 1515(+0.07%) 1514
Changed assets by type (2/7)
            Current     Baseline
CSS 851.78KB (~+0.01%) 851.73KB
JS 8.62MB (+0.03%) 8.62MB

View Job #329 report on app.relative-ci.com

Signed-off-by: Yannick Schaus <github@schaus.net>
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.

Really nice addition!
I just tested it on my prod and it works really well, the separator title trimming is a nice touch ;) Improving these home page cards is definitely something I want to push for the next release and I was actually planning something like that but never got around to it, so you beat me to it 🙂 Just know that it all might be revisited (see #753 for a related discussion). But I'm happy to have this improvement in the meantime while we discuss.

I just had a minor comment on some Stylus syntax that I ended up fixing myself since I had the branch checked out to test.

Thank you very much!

@ghys ghys merged commit 9533fdf into openhab:main Jan 16, 2022
@ghys ghys added enhancement New feature or request main ui Main UI labels Jan 16, 2022
@ghys ghys added this to the 3.3 milestone Jan 16, 2022
@tarag
Copy link
Contributor Author

tarag commented Jan 16, 2022

I'm glad you like it, thanks for the merge. I am really looking forward to have the default model tab pages providing a user-friendly and useful result without too much tweaking and I really think this patch helps in this matter.

There is still one improvement relevant to sorting that I'll try to perform as you can see that the WidgetOrder is not relevant in the Equipment and Properties pages. I'll try to find a way to pull Location related order and have it applied in these pages. WDYT?

My goal is to be able to rely only on the auto-generated model pages - and some canvas pages of course 😉

I had not seen the discussion #753 while searching for related issues. I agree with the ideas discussed there, and I think it could further improve the usability when having many Points. I solve this now with a "maintenance" role where these advanced points are visible but this implies logging-in as another user which is not the best solution. Yet I believe my PR is still relevant even if some of the changes discussed there would be performed as it only deals with labelling.

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.

None yet

2 participants