Skip to content

Add copy entity ID/state/attributes menu button in dev tools/states#4259

Merged
bramkragten merged 13 commits intohome-assistant:devfrom
nicop4:copy-entity-id
Dec 2, 2019
Merged

Add copy entity ID/state/attributes menu button in dev tools/states#4259
bramkragten merged 13 commits intohome-assistant:devfrom
nicop4:copy-entity-id

Conversation

@nicop4
Copy link
Copy Markdown
Contributor

@nicop4 nicop4 commented Nov 21, 2019

Hi
I added a functionality I wished I had numerous times : the ability to quickly copy the entity id without having to manually select text, etc...
For this I added a button next to the "More Info" that will store the ID in the clipboard.
I used the "copy-to-clipboard" package.

It look like this:
HA-copy

That's my first PR for HA so I don't really know the process, please tell me how I proceed now.
I'm not really a front-end developer so any feedback and improvement for a future merge in master are more than welcome.

Also I wasn't really sure how to deal with the new labels I use and their translation in Lokalize , so I only modified the file en.json

@homeassistant
Copy link
Copy Markdown
Contributor

Hi @nicop4,

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!

@bramkragten
Copy link
Copy Markdown
Member

Hi Nicolas,

Thanks for your PR!
Should we maybe move the copy button to the select entity combo box at the top? I'm afraid it will get too crowded with all these buttons in the row...

It is one click extra but a lot cleaner.

Comment thread src/panels/developer-tools/state/developer-tools-state.js Outdated
Comment thread src/panels/developer-tools/state/developer-tools-state.js Outdated
@noxhirsch
Copy link
Copy Markdown
Contributor

noxhirsch commented Nov 21, 2019

Should we maybe move the copy button to the select entity combo box at the top? I'm afraid it will get too crowded with all these buttons in the row...
It is one click extra but a lot cleaner.

But then you'd have to scroll up again after clicking on the entity to copy it. And if you want to copy several entities in a row you have to scroll up and down several times.

@nicop4
Copy link
Copy Markdown
Contributor Author

nicop4 commented Nov 21, 2019

Hi Nicolas,
Thanks for your PR!
Should we maybe move the copy button to the select entity combo box at the top? I'm afraid it will get too crowded with all these buttons in the row...
It is one click extra but a lot cleaner.

But then you'd have to scroll up again after clicking on the entity to copy it. And if you want to copy several entities in a row you have to scroll up and down several times.

Agreed, the goal was to really quickly copy the ID. Any other thoughts about the button placement ?

@bramkragten
Copy link
Copy Markdown
Member

Should we create an overflow menu where we place the items in?

@noxhirsch
Copy link
Copy Markdown
Contributor

Sounds good. Especially does it create a space for more features in the future :)

@nicop4
Copy link
Copy Markdown
Contributor Author

nicop4 commented Nov 21, 2019

Should we create an overflow menu where we place the items in?

Are you talking about a paper menu button ? I'll give it a try ;)

@nicop4
Copy link
Copy Markdown
Contributor Author

nicop4 commented Nov 24, 2019

Hi @noxhirsch @bramkragten
I replaced the button with a drop down menu. It allows multiple actions on the entity, so I offered the ability to copy entity state and attributes as long as ID. It also open space for future actions :)

Here is the new look:
HA-copy-list

Your thoughts ?

@noxhirsch
Copy link
Copy Markdown
Contributor

I love it! (especially the additional copy options you added) The only change I'd make, is to move the more-info icon into the menu as well :)

@nicop4
Copy link
Copy Markdown
Contributor Author

nicop4 commented Nov 24, 2019

Yes I was wondering also if I moved this button too! I'll try to include it as first button:

  • More info
  • Copy entity ID
  • Copy entity state
  • Copy entity attributes

Might also remove entity in the menu labels

@noxhirsch
Copy link
Copy Markdown
Contributor

You're right. entity is not really needed 👍

	modifié :         src/panels/developer-tools/state/developer-tools-state.js
	modifié :         src/panels/developer-tools/state/developer-tools-state.js
@SeanPM5
Copy link
Copy Markdown
Contributor

SeanPM5 commented Nov 24, 2019

This is pretty cool, nice job! I have two very minor suggestions:

  1. I think that for overflow menus typically hass:dots-vertical icon is used, rather than the "hamburger" menu icon (which is more for sidebars and navigation).

  2. And probably remove the exclamation point from "Copied!" on the toast too. It should just say "Copied" or "Copied to clipboard" imo.

@nicop4
Copy link
Copy Markdown
Contributor Author

nicop4 commented Nov 25, 2019

@SeanPM5 Thanks for your feedback, I totally agree !

I have some trouble with the pre-commit hook, which re-indents the source file, which is nice but here it causes problem with the rendering.
In developer-tools-state.js (l.211), I added the attribute id to get its content. My code is:

           <td id="attributes-[[entity.entity_id]]">[[attributeString(entity)]]</td>

But the auto formating hook changes it to this

          <td id="attributes-[[entity.entity_id]]">
                [[attributeString(entity)]]
          </td>

Which should not cause any problem but when displaying the page, the cell with the attributes contains spaces before and after the content which is really ugly (I'll add a screenshot tonight), like if it is some non-breakable spaces.
I disabled the precommit hook so it does not changes the code, but it breaks the CI because of code styling:

/home/travis/build/home-assistant/home-assistant-polymer/src/panels/developer-tools/state/developer-tools-state.js

  211:56  error  Replace `[[attributeString(entity)]]` with `⏎················[[attributeString(entity)]]⏎··············`  prettier/prettier

I don't understand why the spaces are rendered, and how can I change the formatting of the code so that the 'prettifier' doesn't cause this ? I tried many ways of opening and closing tags, with id property but still no luck.

Can someone help or explain me why I have these behaviors ?

Comment thread src/panels/developer-tools/state/developer-tools-state.js Outdated
Comment thread src/panels/developer-tools/state/developer-tools-state.js Outdated
@nicop4
Copy link
Copy Markdown
Contributor Author

nicop4 commented Nov 25, 2019

So I did the little changes we talked about, that is much better this way, thanks for your feedback!

Here is the rendering:
HA-state-list

I still have one minor problem: the drop down list of the menu keeps the previously selected item as you can see on the screen capture. But it seems to be always the case (see the menu to move or delete a card in the UI editor).
I couldn't manage to deselect the item in the menu. According to this issue there is a workaround but I can't get it to work...

@nicop4 nicop4 changed the title Add "Copy entity ID" button in dev tools/states Add copy entity ID/state/attributes menu button in dev tools/states Nov 27, 2019
Comment thread src/panels/developer-tools/state/developer-tools-state.js Outdated
@@ -60,6 +63,10 @@ class HaPanelDevState extends EventsMixin(LocalizeMixin(PolymerElement)) {
height: 24px;
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.

paper-icon-button is no longer used

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.

Don't we use paper-icon-button inside a paper-menu-button ?
Or was it about the paper-icon-buttons I replaced in the list as you suggested in your other comment ?

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.

Yeah you use it inside it, but this style rules are not necessary anymore I think?

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.

Actually there is a 8px padding that shifts the text and it is not aligned with the rest of the table. The height reduces the size of the icon that is too big otherwise.

Comment thread src/panels/developer-tools/state/developer-tools-state.js Outdated
Comment thread src/panels/developer-tools/state/developer-tools-state.js Outdated
@bramkragten
Copy link
Copy Markdown
Member

I miss yarn.lock in your changes, did you run yarn?

@nicop4
Copy link
Copy Markdown
Contributor Author

nicop4 commented Nov 29, 2019

Hi @bramkragten
I did use yarn and I have a yarn.lock, i'm not familiar with this dev environment so I thought it was some kind of local file that was not in the gitignore...
Bu tI will push it with the modifications you proposed :)

use paper-icon-item instead of paper-icon-button and add yarn.lock
Comment thread src/panels/developer-tools/state/developer-tools-state.js Outdated
@bramkragten bramkragten merged commit 4b56db5 into home-assistant:dev Dec 2, 2019
@nicop4
Copy link
Copy Markdown
Contributor Author

nicop4 commented Dec 2, 2019

Glad to see the PR merged 😃 👍

@bramkragten bramkragten mentioned this pull request Dec 4, 2019
bramkragten added a commit that referenced this pull request Dec 9, 2019
bramkragten added a commit that referenced this pull request Dec 9, 2019
…states" (#4337)

* Revert "Add copy entity ID/state/attributes menu button in dev tools/states (#4259)"

This reverts commit 4b56db5.

* Update package.json
bramkragten added a commit that referenced this pull request Dec 9, 2019
…states" (#4337)

* Revert "Add copy entity ID/state/attributes menu button in dev tools/states (#4259)"

This reverts commit 4b56db5.

* Update package.json
@bramkragten bramkragten mentioned this pull request Nov 9, 2020
9 tasks
@jazzmonger
Copy link
Copy Markdown

What ever happened to this PR? It's so much more useful than just the 'info' icon...

@nicop4
Copy link
Copy Markdown
Contributor Author

nicop4 commented Dec 24, 2020

Yeah it almost made it to a release but it seems it had a big impact on performance with "large" amount of entities. I never took a second look at it as I am not a front-end expert, but I still wish to have it.
I might restart it or may be reduce the number of functions of this menu. If you think you can help me we can get in touch!

@jazzmonger
Copy link
Copy Markdown

Im just a user that could really use this!! copying this stuff is a pain on a mobile device.

@github-actions github-actions Bot locked and limited conversation to collaborators Jul 5, 2022
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.

6 participants