home-assistant / android

:iphone: Home Assistant Companion for Android
https://companion.home-assistant.io/
Apache License 2.0
2.29k stars 636 forks source link

Add UI for editing the list of existing NFC tags plus a "Name" field to label them #4709

Closed NoRi2909 closed 1 week ago

NoRi2909 commented 1 week ago

Home Assistant Android app version(s): 2024.10.1-full (beta) Android version: 15 Device mode: Pixel 6

Every NFC tag you create with the Companion app ends up in the list that shows up under the 'Tag' type trigger.

As these are all just GUIDs this list can become very hard to handle just with a dozen of tags created. So there needs to be at least a UI to clear this of all the Tags that no longer exist.

This is currently missing, and there is also not even a documentation which text file to edit as a workaround.

In addition it would be absolutely awesome if you at least added a Name field when writing a tag that would show up before the GUID in the 'Tag' trigger menu. The GUID would then still be created automatically but no longer be editable and only a secondary column in the Tags list.

I know that you can change the identifier before writing so it can serve this purpose, but nobody will figure out a good naming scheme that can last for years when just getting started with this feature.

Being able to also edit the Name later instead of altering the GUID when writing should be the better solution here.

jpelgrom commented 1 week ago

The interface you describe exists in Settings > Tags.

The companion app settings don't have additional UI because this is all that is written to the tags.

NoRi2909 commented 1 week ago

@jpelgrom Thanks for the quick reply. Sorry that I missed the reference to Settings > Tags on the Help page because I read it on the smartphone. I should always copy the URL to my desktop to better browse it on the big screen…

NoRi2909 commented 1 week ago

I just see that part of what I suggested above already exists. It's just not really obvious …

The ID is copied to that Name field on the server and I can in fact edit it there without altering the ID.

Edit: My feature request stemmed from the fact that there are two different routes on the mobile device for writing an NFC tag:

  1. Under Settings > Tags tapping the "Add tag" button allows me to do exactly what I suggested, i.e. setting the name before writing the tag.

  2. Under Settings > Companion App > NFC Tags tapping on "Write NFC tag" does no offer naming the tag before writing.

So there is some inconsistency that caused me to ask for a feature that already exists, but in a different part of Settings.

NoRi2909 commented 1 week ago

All of my comments are currently fueled by trying to correctly translate the Android App into German. There was so much misleading stuff in there, it can really confuse when trying to make sense of everything.

There is a small inconsistency between the mobile app and the server:

The server talks about "Tag ID" while the mobile app says "Identifier". While this obviously the same in English, in German this becomes "ID" and "Kennung" which add unnecessary confusion.

So it would be great if you changed "Identifier" into "ID" or "Tag ID" in these strings:

 nfc_write_tag_instructions
 nfc_trigger_summary
 nfc_tag_identifier
 nfc_write_tag_enter_identifier
 nfc_write_tag_change

Then this would match all the user sees in the server UI. And I could translate this consistently into German – well, just keep "Tag-ID" (with a dash) there, too.

NoRi2909 commented 1 week ago

And one more bug to fix:

 nfc_trigger_summary
 Select the 'Tag' trigger type and choose the identifier in the visual editor, or use the following YAML code:

Here it says "choose the identifier" but you chose the Tag by its Name (which just happens to be a copy of the ID by default).

NoRi2909 commented 1 week ago

And another subtle bug in the workflow, easy to fix by adding a single word.

This is in Home Assistant - frontend:

  ui::panel::config::tag::create_automation
  "Create automation with tag"

This shows up when hovering the mouse pointer over this icon in the Tags list on the server:

Create automation

Should be "Create automation with this tag" instead, otherwise it becomes hard to understand in translations.

dshokouhi commented 1 week ago

This is in Home Assistant - frontend:

the app has no control over these screens as they are not owned by the app. Please create a HA frontend issue for that.

NoRi2909 commented 1 week ago

@dshokouhi OK, I thought you could also do such an addition in the frontend code. Wanted to avoid to file a bug report for a missing word …

Then let's stick to the app issues I mentioned above this post.

dshokouhi commented 1 week ago

This change may require HA core changes. Not entirely sure the API will accept a NFC tag name? Looks like the external bus only expects the ID field.

https://github.com/home-assistant/android/blob/505e3b0d7f764cc6d97f54e2a19b399afa1e1162/app/src/main/java/io/homeassistant/companion/android/webview/WebViewActivity.kt#L158-L170

NoRi2909 commented 1 week ago

@dshokouhi I just edited my post above after figuring this out:

There are two different routes on the mobile device for writing an NFC tag:

  1. Under Settings > Tags tapping the "Add tag" button allows me to do exactly what I suggested, i.e. setting the name before writing the tag via the mobile app's native UI. Edit: But it does not work.

  2. Under Settings > Companion App > NFC Tags tapping on "Write NFC tag" does not offer naming the tag before writing.

So it the web UI does offer setting a name but cannot pass it through the app.

NoRi2909 commented 1 week ago

OK, this is developing into an interesting bug …

Steps to reproduce:

  1. On mobile go to Settings > Tags and tap the "Add tag" button.
  2. Enter a name, tap "Create and write"
  3. Proceed writing the tag.

Now you get this - the name is shown (I used "Test"):

Screenshot 09 10 2024 16 00

Now check on your desktop or reload the page. The name will be gone because it was not really saved on the server:

Screenshot 2

We just got the ID that the server copied into the Name field.

jpelgrom commented 1 week ago

If you can reproduce the issue as mentioned, please report it in the frontend repository.

Tags do not hold names on them. You can assign names on a server but the app screen you mention is not contacting servers or designed to manage tags like you want.

NoRi2909 commented 1 week ago

@jpelgrom Tried again on Desktop, I can reproduce the issue there with QR code tags, too: After refreshing the page the name that is shown at first is gone.

I will file a bug in the frontend repository, seems to be 100% in that territory.

Thank you very much for pointing me in the right direction with this issue.

I have one last related question regarding NFC tags so I can get the German translation in the Android app right. I'm through with everything else, definitely over 1000 fixes … It was quite a mess as English isn't really very precise without the full context visible, so the previous translators created a lot of misleading and inconsistent terms.

I did not see the following four strings show up anywhere in the UI, yet. Perhaps you can give me just a quick hint how they are meant:

 nfc_edit_fragment_label
 NFC Edit Fragment

 nfc_write_fragment_label
 NFC Write Fragment

 nfc_read_fragment_label
 NFC Read Fragment

 nfc_welcome_fragment_label
 NFC Welcome Fragment

Are "Read Fragment" etc. all nouns, denoting some part of the tag that is written, or do I read those as verbs (i.e. editing, reading, writing, welcoming ?)

I searched the web for any good information but wasn't successful.

dshokouhi commented 1 week ago

I did not see the following four strings show up anywhere in the UI, yet.

those strings appear to have been previously removed, maybe they were never removed from Lokalize? 🤔

https://github.com/home-assistant/android/blob/master/common/src/main/res/values/strings.xml

this file contains all the current strings used

NoRi2909 commented 1 week ago

Here's a screenshot from Localize:

Screenshot 2024-10-09 18 48 05

If you remove them from Lokalize that's the best solution for my problem. :-)

dshokouhi commented 1 week ago

Thanks I have updated our setting there, it should be cleaned up the next time we merge a PR

NoRi2909 commented 1 week ago

@dshokouhi @jpelgrom Thank you both so much for your patience and help.

I'm looking forward to the next beta to see all the localization changes live in the UI. 99% should be OK, but some button texts don't fit in the current release version so it will be very helpful to see those fixes live soon.