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

Configuration option to hide item state in main UI list widgets #1285

Merged
merged 2 commits into from
Jan 24, 2022

Conversation

tarag
Copy link
Contributor

@tarag tarag commented Jan 19, 2022

With a standard label card it is possible to replace the item label using the label parameter. On list items, there is the possibility to add labels before or after the item state, but never to modify the item state itself. There is now way to override this even using a transformation for pure command items because the items always remain NULL. For these items, I found no way to hide the "NULL" label in the UI while preserving the built-in commands capability.

With this patch, it is possible to take full benefit of the oh-label-item automatic creation (giving access to the list of possible commands to be sent for example for a Number item with commandDescription metadata set), but not display the ugly "NULL" label.

Some related issues:
https://community.openhab.org/t/hide-item-state-for-group-items-in-widget/117121
https://community.openhab.org/t/how-to-get-rid-of-item-states-undef-or-null-in-main-ui/126013

I use this for example to command KNX scenes (via the popup list), which are items without a valid state. With the single parameter set, the default rendering is perfect.

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

relativeci bot commented Jan 19, 2022

Job #340: Bundle Size — 10.67MB (~+0.01%).

dd582e1 vs ab31798

Changed metrics (2/8)
Metric Current Baseline
Initial JS 1.67MB(+0.05%) 1.67MB
Cache Invalidation 15.78% 21.32%
Changed assets by type (1/7)
            Current     Baseline
JS 8.62MB (~+0.01%) 8.62MB

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

Comment on lines 3 to 4
<div slot="after">
{{ context.store[config.item].displayState || context.store[config.item].state }}
{{ config.hideState ? '' : context.store[config.item].displayState || context.store[config.item].state }}
Copy link
Member

Choose a reason for hiding this comment

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

Fair enough to have a parameter, but there was another bug that we could solve and maybe address this use case at the same time.
The list items, including the label item, all have an after parameter to set something on the slot of the same name. Obviously when it's set you'd want it to replace the default (as is the case for oh-label-card). But here we end up with 2 divs added in the after slot which results to this:

image

So I would suggest this instead:

<div slot="after" v-if="config.after !== undefined">

that way you can get rid of the "after" label altogether by setting after to e.g. ="", no additional parameter needed. Would that work for you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I would suggest this instead:

<div slot="after" v-if="config.after !== undefined">

that way you can get rid of the "after" label altogether by setting after to e.g. ="", no additional parameter needed. Would that work for you?

I am trying to achieve this:

hide_state

It work easily by hiding the NULL state (my PR) and then adding some text in after field with Appel/Enreg. value. With your proposal, I can't do both at the same time, and furthermore the state with a scene value might be displayed later-on which I'd like to avoid since they do not represent a meaningful state.

Yet, I found that it hide plenty of lingering NULLs and looks cleaner, but maybe there should be a localizable label displayed instead such as N/A or just a dash to indicate that something might come back.

Copy link
Member

Choose a reason for hiding this comment

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

It work easily by hiding the NULL state (my PR) and then adding some text in after field with Appel/Enreg. value. With your proposal, I can't do both at the same time, and furthermore the state with a scene value might be displayed later-on which I'd like to avoid since they do not represent a meaningful state.

Why? My point above was that since oh-label-item is based on oh-list-item there are 2 elements in the "after" slot if you define the after property on a oh-label-item - one defined by the after property on the "parent" oh-list-item and one defined by the oh-label-item component itself.

image

<div class="item-content">
  <div class="item-media"><img onload="this.classList.remove('no-icon')" onerror="this.classList.add('no-icon')"
      src="/icon/house?format=svg&amp;anyFormat=true" style="width: 32px; height: 32px;"></div>
  <div class="item-inner">
    <div class="item-title-row">
      <div class="item-title">Scene General</div>
      <div class="item-after">
        <span class="">should replace the state if defined</span>
        <div>Bedtime</div>
      </div>
    </div>
    <!---->
    <!---->
  </div>
</div>

With my proposal if you define the after property at all (even to null or an empty string) on a oh-label-item then the second div (the "state" one) should not be rendered at all, effectively making oh-label-item behaving like a oh-list-item (btw, the sole purpose of a oh-label-item over a oh-list-item is to display the state of an item). If you want to define what's displayed instead of the state you can use an expression in the after property. Or am I missing something?

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 the explanation. I'm not really familiar with all these slots things...

With my proposal if you define the after property at all (even to null or an empty string) on a oh-label-item then the second div (the "state" one) should not be rendered at all, effectively making oh-label-item behaving like a oh-list-item (btw, the sole purpose of a oh-label-item over a oh-list-item is to display the state of an item). If you want to define what's displayed instead of the state you can use an expression in the after property.

You are absolutely right. That's exactly what I want to achieve and thanks to you I found out that I can indeed achieve the same goal with a regular oh-list-item. An important difference is that to use a regular oh-list-item I have to define additional listWidget metadata to reach the same goal (value: oh-list-item, then inside config action: options, actionItem: the item name again...) whereas with a correction I can limit to only the configuration of : after: my right label. Multiplied by the number of items this changes the deal quite a lot, maybe that's why I had not even found this solution.

Or am I missing something?

Maybe the misunderstanding is that I did not notice a typo in your proposal at first, because the behaviour you describe is achieved by implementing this in oh-label-item.vue: <div slot="after" v-if="config.after === undefined"> (not with !== test). Isn't it what you meant? It now does seem obvious and it works perfectly with this correction.

Copy link
Member

Choose a reason for hiding this comment

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

(not with !== test). Isn't it what you meant?

Yes 🤦

Signed-off-by: Gautier Taravella <tarag@mailbox.org>
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.

Thanks for this - and it fixes the "bug" with the 2 after slot elements as well 👍

@ghys ghys merged commit 39fa259 into openhab:main Jan 24, 2022
@ghys ghys added bug Something isn't working main ui Main UI labels Jan 24, 2022
@ghys ghys added this to the 3.3 milestone Jan 24, 2022
@tarag tarag deleted the hide-item-state branch May 23, 2022 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working main ui Main UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants