-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Add Voice assistant column to data tables #28785
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @kristel030
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
MindFreeze
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How will multiple assistants be handled? There should be some spacing between the icons I guess.
|
@MindFreeze thanks for the valuable feedback. Regarding spacing in case of multiple assistants. Visually I don't believe there's a spacing issue between the images. But I am faced with a column width issue when all three assistant logos need to be rendered, as the third logo overlaps the text in the next column. Do you maybe have any suggestions on how to fix this? Or would it be acceptable to leave as is, as supporting three voice assistant flavours seems to be a kind of edge case. |
|
Addressed all review comments. New version reuses existing implementation of rendering the logo of relevant voice assistants. |
|
Sorry, but isn’t it better to show names of assistants instead of icons? In this case there will be no overflow issue, ellipsis will be shown in case of many assistants (+ tooltip may be show with a full list), also a “voice assistant” filter can be added. |
I agree but we already have this as icons on the Voice Assistants page in the Expose tab. So lets just make this consistent with that first. Otherwise the scope of the PR will expand. |
MindFreeze
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the overflow still an issue? If yes, check how the voice assistants page handles it
|
Sorry for another intrusion, is it possible to make it sortable? Also, adding a filter might be also useful. Not sure if it is possible with “icon” presentation. |
@ildar170975 From a usability perspective I'm completely with you. However, this is my first code contribution to this code base, and I haven't yet been able to find out how to make sorting or filtering work. I think it is wise to leave the scope of this PR as it is (to keep it manageable) and introduce sort/filter as a separate PR. |
The voice assistant page displays multiple icons below each other instead of next to each other. I have decided to leave this behaviour as it is. I personally prefer displaying next to each other, but first tests revealed quite some challenges in guaranteeing proper layout, so best to not tinker with that right now. |
MindFreeze
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current voice assistants page uses
showNarrow: true,
sortable: true,
filterable: true,
minWidth: "160px",
maxWidth: "160px",
type: "flex",
Please make all the new columns use the same options for consistency. You have various sizes now and it doesn't seem intentional
All minWidth/maxWidth for all data tables that get a new 'Voice assistant' column with this PR are now set at 100px. Reasing being is that the 160px value claims more horizontal space then is actually needed, which comes at the expense of other columns. So I believe we have 3 options now:
|
This is fine as long as it works visually but 1 of the new ones had 160px so I thought it was a mistake |
|
@MindFreeze Wrap-up:
Thanks for being my 'conscience' in this PR, your review remarks helped me to better understand the code structure and improve the code quality. Can't wait to see my very first contribution appearing in an upcoming release. |
MindFreeze
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @kritsel ! Great work! FYI it's too late for the January release so this will go in the February one.

Proposed change
A column named 'Voice assistants' is added to the Entities, Helpers, Automations, Scenes and Scripts overview pages. The column shows the brand icon(s) of the voice assistant(s) an entity, helper, etc. is exposed to. This makes it easier for users to see which entities, etc. are actually controlable via a voice assistant.
The 'Voice assistants' column is hidden by default (use the 'Customize table' button above a data table to unhide) and it is not sortable.
Type of change
Example configuration
N/A
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed: