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 new options to manage the header history icon #7

Merged
merged 3 commits into from
Jan 6, 2024

Conversation

elchininet
Copy link
Collaborator

@elchininet elchininet commented Jan 5, 2024

This pull request adds new options to manage the history icon on the header of more-info dialogs:

hide_header_history_icon

Hides the header history icon.

custom_more_info:
  hide_header_history_icon:
    # It is also possible to set a boolean value for everything
    # all: true
    by_glob:
      ...
    by_domain:
      ...
    by_device_class:
      ...
    by_entity_id:
      ...

unhide_header_history_icon

Unhides the header history icon.

custom_more_info:
  unhide_header_history_icon:
    # It is also possible to set a boolean value for everything
    # all: true
    by_glob:
      ...
    by_domain:
      ...
    by_device_class:
      ...
    by_entity_id:
      ...

auto_hide_header_history_icon

Hides the header history icon automatically if the history and logbook have been hidden form that entity.

custom_more_info:
  auto_hide_header_history_icon: true

Note: as part of this pull request, the home-assistant-query-selector dependency has been updates to its major version.

@Mariusthvdb
Copy link
Owner

cant wait to test this, thank you!
does this also auto hide the button if the user uses

hide_history:
  by_xxx:

Because that would make sense?
or, would you use it the other way around.

tbh, it's a bit confusing to me having both. But maybe as a dev you see the added value of having a button to hide and a section to hide (while both being for history)

@elchininet
Copy link
Collaborator Author

cant wait to test this, thank you!

Please, test it deeply. As you see, it could be seen as just three options but it introduces a lot of code and complexities.

does this also auto hide the button if the user uses
hide_history:
by_xxx:

No, hiding the history doesn‘t hide the button because that button also contains the logbook panel, if one hides both then the button hides (if one uses auto_hide_header_history). The name is confusing though because Home Assistant named it as "History" but it contains also "Logbook".

image

tbh, it's a bit confusing to me having both.

Well, that was you requested earlier ;)

Normally, I do not like the "auto" things (better if it is through an option) but anything is possible. It is your call. Just let me know what would be the best way and I can implement it here in this pull request.

@Mariusthvdb
Copy link
Owner

Mariusthvdb commented Jan 5, 2024

if one hides both then the button hides (if one uses auto_hide_header_history).

sorry, yes, that is exactly what I meant, and I can confirm this to work nicely now.
set it to auto-hide and hide both logbook and history on an entity makes the icon disappear.

which brings me to the naming of the option. should we no add the concept of the 'button' or 'icon' to that, just like your PR does?

I feel it would help in expressing what the option does. the header-history-item on itself is a bit vague, but adding either of those 2 (button/icon) or maybe even 'tab' would be helpful.

hide-header-history-tab, or hide-history-button, something like those?

do we also have an 'all' setting for that? or would it be simplest to do that with a glob:

hide_header_history:
    by_glob:
      - '*.*"

it would allow to hide globally, and then unhide only on a certain useful ones

had a quick check at Kiosk-mode:

Scherm­afbeelding 2024-01-05 om 19 19 17

and look at the description: Hide history icon ;-) I believe the description to be more succinct than the option naming..

@Mariusthvdb
Copy link
Owner

Mariusthvdb commented Jan 5, 2024

found an issue:

hide_header_history:

  by_domain:
    - input_datetime
    - automation
    - script
    - sun

doesnt work for sun. Nor does:

  by_entity_id:
    - sun.sun

@elchininet
Copy link
Collaborator Author

which brings me to the naming of the option. should we no add the concept of the 'button' or 'icon' to that, just like your PR does?

Makes sense to me, which one would you choose?

do we also have an 'all' setting for that? or would it be simplest to do that with a glob

I forgot about it, no need to, it should be easy to implement.

found an issue:
doesnt work for sun. Nor does:

I need to check how the layout is on that entity, maybe it is different.

@Mariusthvdb
Copy link
Owner

Mariusthvdb commented Jan 5, 2024

which brings me to the naming of the option. should we no add the concept of the 'button' or 'icon' to that, just like your PR does?

Makes sense to me, which one would you choose?

even HA cant make up their mind ;-)

Scherm­afbeelding 2024-01-05 om 21 06 57

but I believe we should go for the concept 'icon', since it is not a true button at all.

hide-header-history-icon would be perfect, albeit a bit long. But because of that it stands out nicely against the hide-history option

do we also have an 'all' setting for that? or would it be simplest to do that with a glob

I forgot about it, no need to, it should be easy to implement.

cool, no hurry, could be a separate PR if you'd like and merge this first

found an issue:
doesnt work for sun. Nor does:

I need to check how the layout is on that entity, maybe it is different.

O sorry, I completely forgot to do that myself.. I figured it would be because of not having a unique_id so not UI configurable...

which also applies to the domain: group (yaml configured, or via automation). History icon remains in view in spite of:

hide_header_history:

  by_domain:
    - input_datetime
    - automation
    - script
    - cover
    - group

inspector:
Scherm­afbeelding 2024-01-05 om 21 24 09

@elchininet
Copy link
Collaborator Author

I don't see anything weird with the layout, I need to check the properties of the elements in those entities.

@elchininet elchininet changed the title Add new options to manage the header history button Add new options to manage the header history icon Jan 6, 2024
@elchininet
Copy link
Collaborator Author

@Mariusthvdb,
I have pushed the rename of the options and also the possibility of a boolean value named all, if it is set in true it hides (or unhides) the icon for any entity.

@elchininet
Copy link
Collaborator Author

Also, the sun.sun entity should be fixed. Try also with other entities to see if you find failures in them.

@Mariusthvdb
Copy link
Owner

Mariusthvdb commented Jan 6, 2024

Also, the sun.sun entity should be fixed. Try also with other entities to see if you find failures in them.

nice! can confirm that using the new hide option for the icon like this:

auto_hide_header_history_icon: true

hide_header_history_icon:

  by_domain:
    - input_datetime
    - automation
    - script
    - cover
    - group

  by_entity_id:
    - sun.sun

removes the icon for all groups and the sun.sun correctly! so cool this, thanks for your effort!

I take it this is also changed to use the new _icon suffix?

auto_hide_header_history_icon: true

btw, what was the fix for it?

Copy link
Owner

@Mariusthvdb Mariusthvdb left a comment

Choose a reason for hiding this comment

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

all seems well now, so marking approved!

hurray

@Mariusthvdb Mariusthvdb merged commit df179ce into main Jan 6, 2024
2 checks passed
@elchininet elchininet deleted the add_new_options_for_header_history_button branch January 6, 2024 20:18
@elchininet
Copy link
Collaborator Author

I take it this is also changed to use the new _icon suffix?

Yes

@elchininet
Copy link
Collaborator Author

Make a release, so I can submit a pull request to add the plugin in HACS. The sooner, the best.

@Mariusthvdb
Copy link
Owner

consider that done, please check

elchininet pushed a commit that referenced this pull request Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants