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

Wrap dict attributes #18290

Merged
merged 2 commits into from
Oct 20, 2023
Merged

Conversation

joshmcrty
Copy link
Contributor

Proposed change

This lets the <pre> element used for dict_attr wrap long text instead of breaking out of its container when viewing sensor attributes, for example, in a more info dialog. Simply wrapping the text will ensure that all of the information is visible, but it could become difficult to read for very large blocks of text as seen in #18027.

This also removes the default margins on the <pre> element to be consistent with how other attribute values are displayed.

A couple of other ways to fix this could be to use text-overflow: ellipsis and either overflow: hidden or overflow-x: auto, or even offer some kind of "view more" button that pops up a separate, larger dialog with just that text to give it more room to be viewed.

Before:
Screenshot 2023-10-19 at 12 08 22 PM

After:
Screenshot 2023-10-19 at 12 40 58 PM

With text-overflow: ellipsis and overflow-x: auto:
Screenshot 2023-10-19 at 12 41 26 PM

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

This is borrowed from the issue linked below

template:
  - sensor:
      - name: test_long_attr
        state: ok
        attributes:
          dict_attr_long: >-
            {% set LONG_VALUE_1 = "Morocco lies close to the Azores–Gibraltar Transform Fault." %}
            {% set LONG_VALUE_2 = "Morocco lies close to the Azores–Gibraltar Transform Fault." %}
            {%- set DICT = {
              'value_1': LONG_VALUE_1,
              'value_2': LONG_VALUE_2
              } -%}
            {{DICT}}

Additional information

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

@bramkragten bramkragten merged commit 16766f8 into home-assistant:dev Oct 20, 2023
14 checks passed
@joshmcrty joshmcrty deleted the wrap-dict-attributes branch October 24, 2023 15:08
@ildar170975
Copy link
Contributor

ildar170975 commented Nov 12, 2023

@joshmcrty
Thank you for the fix!
But can you fix a font too?
#17932 (comment)

May be add "font-family" here?

    pre {
      margin: 0;
      white-space: pre-wrap;
      font-family: inherit;
    }

@Mariusthvdb
Copy link
Contributor

tbh, I am no sure whether it is the fact we have a longer attribute, or we have an attribute in a dictionary . If the former, we might need to open a dedicated issue for this font issue?

@ildar170975
Copy link
Contributor

longer attribute

For this case it does not show a monospace font.

dedicated issue

I'll probably open a PR - if the author of the latest change will not do it himself.

@ildar170975
Copy link
Contributor

ildar170975 commented Nov 13, 2023

@joshmcrty
I wonder why these styles for pre were not used in ha-attributes-value ?

        pre {
          font-family: inherit;
          font-size: inherit;
          margin: 0px;
          overflow-wrap: break-word;
          white-space: pre-line;
        }

These attributes give both a correct font & a wrapping.

@joshmcrty
Copy link
Contributor Author

@ildar170975 thanks for looking into this further. I'm guessing that when ha-attributes-value was created in #17249 the pre styles were left behind in ha-attributes. I created this PR while only focused on the wrapping issue. I wasn't aware that it had previously also matched the standard font. I thought the monospace font was intentional. I can create another PR that moves all of the prior styles to the correct location. I don't think the pre styles are needed anymore in ha-attributes.

@ildar170975
Copy link
Contributor

I don't think the pre styles are needed anymore in ha-attributes

I also wondered what that pre code is doing in ha-attributes...
So, shall we simply add SAME code to the ha-attributes-value:

          font-family: inherit;
          font-size: inherit;
          margin: 0px;
          overflow-wrap: break-word;
          white-space: pre-line;

even may be instead of the code added by you?
I made a quick test - just opened Code Inspector in Chrome & then added those styles to the pre element, and (as said it before) this gave both a correct font & a wrapping.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

more-info: displaying "dictionary" attributes is broken
4 participants