Skip to content

Rework ZHA group adds and removes#5602

Merged
bramkragten merged 14 commits into
home-assistant:devfrom
dmulcahey:dm/rework-zha-group-adds-removes
May 12, 2020
Merged

Rework ZHA group adds and removes#5602
bramkragten merged 14 commits into
home-assistant:devfrom
dmulcahey:dm/rework-zha-group-adds-removes

Conversation

@dmulcahey
Copy link
Copy Markdown
Contributor

Proposed change

This PR fixes the grouping UI so that it works based on endpoints and not based on devices.

REQUIRES: home-assistant/core#34583

A full explanation can be found in the description of the linked PR.

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

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

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.

If user exposed functionality or configuration variables are added/changed:

@dmulcahey dmulcahey changed the title Dm/rework zha group adds removes Rework zha group adds removes Apr 23, 2020
@dmulcahey dmulcahey changed the title Rework zha group adds removes Rework ZHA group adds removes Apr 23, 2020
@dmulcahey dmulcahey changed the title Rework ZHA group adds removes Rework ZHA group adds and removes Apr 23, 2020
@dmulcahey dmulcahey force-pushed the dm/rework-zha-group-adds-removes branch from 2e1fa05 to b91ca92 Compare April 27, 2020 11:56
@dmulcahey dmulcahey requested a review from bramkragten May 4, 2020 14:11
Comment on lines +123 to +125
html`<div
class="mdc-data-table__cell table-cell-text"
>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why create multiple cells in a cell?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

image

It's how I got the associated entities to render correctly like this, including the elided text. Does that make sense?

Copy link
Copy Markdown
Member

@bramkragten bramkragten May 8, 2020

Choose a reason for hiding this comment

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

We should not do that, also, what happens when there are 12 entities?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We don't have anything where that would be possible yet... if it comes to pass and we have a situation like that I can elide the list too. Do you have any suggestions?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add style="overflow: hidden; text-overflow: ellipsis;" instead of the classes.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So it is not possible that there are more than 3 entities associated with a groupable device?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Updated it to make sure:
image

Comment thread src/panels/config/zha/zha-group-page.ts Outdated
Comment thread src/panels/config/zha/zha-group-page.ts Outdated
@dmulcahey dmulcahey force-pushed the dm/rework-zha-group-adds-removes branch from b91ca92 to 62257ed Compare May 4, 2020 21:02
@dmulcahey dmulcahey force-pushed the dm/rework-zha-group-adds-removes branch from b58b73a to 8b30334 Compare May 8, 2020 12:06
@dmulcahey
Copy link
Copy Markdown
Contributor Author

@bramkragten any chance we can get this done before the beta? If not, I'll have to revert the PR on core.

@bramkragten
Copy link
Copy Markdown
Member

Yeah should not be a problem

Comment thread src/panels/config/zha/zha-device-endpoint-data-table.ts Outdated
@bramkragten bramkragten merged commit 577a21f into home-assistant:dev May 12, 2020
@bramkragten bramkragten mentioned this pull request May 12, 2020
@lock lock Bot locked and limited conversation to collaborators May 20, 2020
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.

3 participants