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

Label and style all UID elements consistently #2336

Merged
merged 4 commits into from Feb 18, 2024

Conversation

jimtng
Copy link
Contributor

@jimtng jimtng commented Feb 10, 2024

Resolve #2335

Old Label New Label Note
ID Page ID
ID Widget ID
Unique ID Rule ID
Unique ID Transformation ID This is for the last part of the Transformation ID
Unique ID Transformation UID This was labelled the same as above but hidden during creation.
Identifier Thing ID
Channel Identifier Channel ID This is for the last part of the Channel UID

Create a new Thing

  • Changed Unique ID => Thing ID
  • To be consistent with Add Channel and Add Transformation, the full Thing UID is not shown when creating a new thing
image

View an existing Thing

  • Changed Identifier => Thing UID
  • The copy UID icon is moved to the end of the UID, to be consistent with all the other Copy UID icons
image

Add a new channel

  • Changed Channel Identifier => Channel ID
image

Copy channel

image

View an existing channel

  • Enable the copy UID icon even when the input is disabled
image

Add an Item

  • Previously, a default item was pre-set to NewItem, making the placeholder not visible, and if the given default is erased, an error message is immediately displayed, so the placeholder is never visible either way.
  • This PR changed so that no default is set for either the item name or the item label.
  • The placeholder for the item name now indicates that it is a Unique ID for the item and that it cannot be changed afterwards.
  • Previously a default label was set to 'New Item', making the label placeholder invisible.
  • The label is now empty by default. This causes the placeholder to show which provides further hints about the Label vs the Item Name.
image

Edit/show an existing item

image

Create a new transformation

  • Unique ID => Transformation ID (Only the last part of the transformation uid)
image

Show an existing transformation

  • Unique ID => Transformation UID (The full UID)
image

Show Page ID

  • ID => Page ID
image

Show existing Rule

  • Unique ID => Rule ID
image

Add a new rule

  • Unique ID => Rule ID
image

@jimtng jimtng requested a review from a team as a code owner February 10, 2024 07:10
Copy link

relativeci bot commented Feb 10, 2024

Job #1589: Bundle Size — 11.01MiB (~+0.01%).

da0e865(current) vs 3f02477 main#1585(baseline)

Warning

Bundle contains 19 duplicate packages – View duplicate packages

Bundle metrics  Change 1 change
                 Current
Job #1589
     Baseline
Job #1585
No change  Initial JS 1.84MiB 1.84MiB
No change  Initial CSS 607.89KiB 607.89KiB
Change  Cache Invalidation 17.93% 23.24%
No change  Chunks 220 220
No change  Assets 242 242
No change  Modules 3087 3087
No change  Duplicate Modules 164 164
No change  Duplicate Code 1.77% 1.77%
No change  Packages 150 150
No change  Duplicate Packages 18 18
Bundle size by type  Change 1 change Regression 1 regression
                 Current
Job #1589
     Baseline
Job #1585
Regression  JS 9.2MiB (~+0.01%) 9.2MiB
Not changed  CSS 889.37KiB 889.37KiB
Not changed  Fonts 526.1KiB 526.1KiB
Not changed  Media 295.6KiB 295.6KiB
Not changed  IMG 140.74KiB 140.74KiB
Not changed  HTML 1.24KiB 1.24KiB
Not changed  Other 871B 871B

View job #1589 reportView jimtng:uniform-entity-id-labelli... branch activityView project dashboard

@mvalla
Copy link

mvalla commented Feb 10, 2024

For consistency also the doc page should be updated:

itemname -> itemid

https://www.openhab.org/docs/configuration/items.html

@jimtng
Copy link
Contributor Author

jimtng commented Feb 10, 2024

For consistency also the doc page should be updated:

itemname -> itemid

I think this really needs a serious discussion from everyone. I feel that this change in terminology is going to be a fundamental change that everyone needs to be aware of and be on board with. @openhab/core-maintainers

The start of this discussion is here: https://community.openhab.org/t/item-editor/153739/23

@jimtng jimtng force-pushed the uniform-entity-id-labelling branch 5 times, most recently from cf3d936 to b9f34de Compare February 10, 2024 09:15
@jimtng
Copy link
Contributor Author

jimtng commented Feb 10, 2024

@mvalla I've updated the original post with more details + screenshots

@jimtng
Copy link
Contributor Author

jimtng commented Feb 10, 2024

I'd like to hear your opinion too, @rkoshak

@wborn
Copy link
Member

wborn commented Feb 10, 2024

It is probably also part of REST/WebSocket APIs and code in core so if you want to be consistent those should also be updated. Those would be breaking API changes.

@mvalla
Copy link

mvalla commented Feb 10, 2024

It is probably also part of REST/WebSocket APIs and code in core so if you want to be consistent those should also be updated. Those would be breaking API changes.

Is it really necessary?
One thing is improve labels in the UI and correct the mess that is evident from the table in the first comment (basically every UI page uses a different way to label the object ID…)

Another thing is now refactor the API that has of course huge implications with all the apps…

@jimtng jimtng marked this pull request as draft February 10, 2024 20:30
@jimtng jimtng force-pushed the uniform-entity-id-labelling branch 3 times, most recently from 48cc51c to c39428a Compare February 11, 2024 13:49
@jimtng jimtng marked this pull request as ready for review February 11, 2024 14:02
@ghys
Copy link
Member

ghys commented Feb 11, 2024

image

Have you checked whether this "Transformation UID" label wouldn't be truncated on mobile devices?

@jimtng
Copy link
Contributor Author

jimtng commented Feb 11, 2024

Have you checked whether this "Transformation UID" label wouldn't be truncated on mobile devices?

I made a transformation with a super long ID

This is how it looks like right now, before this PR. It got truncated.

image

With this PR, still truncated, but the copy icon is there
image

But yes, perhaps making it into a full line like before would give it that bit extra space. I'll push a change shortly.

@jimtng
Copy link
Contributor Author

jimtng commented Feb 11, 2024

I've made transformation uid into a separate line just like channel uid. It's still not ideal/foolproof. Perhaps a separate PR to clean up the styling for all the potentially-long fields.

@florian-h05
Copy link
Contributor

@jimtng Can you please resolve conflicts?

@jimtng
Copy link
Contributor Author

jimtng commented Feb 18, 2024

I've made transformation uid into a separate line just like channel uid. It's still not ideal/foolproof. Perhaps a separate PR to clean up the styling for all the potentially-long fields.

What's the preferred method: merging main like what I just did here, or rebase and force push?

@florian-h05
Copy link
Contributor

Rebase and force push.

@jimtng
Copy link
Contributor Author

jimtng commented Feb 18, 2024

Rebase and force push.

Would you like me to do that now?

@florian-h05
Copy link
Contributor

If we then get rid of the merge commit, yes, please.

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
@jimtng
Copy link
Contributor Author

jimtng commented Feb 18, 2024

Rebased. There were some minor changes (removal of unnecessary changes) too.

Copy link
Contributor

@florian-h05 florian-h05 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

Just two minor comments I will take care myself.
I will also align the placeholders and notes used across the UI.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05 florian-h05 added enhancement New feature or request main ui Main UI labels Feb 18, 2024
@florian-h05 florian-h05 added this to the 4.2 milestone Feb 18, 2024
@florian-h05 florian-h05 changed the title Label all UID elements consistently Label and style all UID elements consistently Feb 18, 2024
@@ -6,12 +6,13 @@
info="Required. Note: cannot be changed after the creation"
required validate pattern="[A-Za-z0-9_\-]+" error-message="Required. A-Z,a-z,0-9,_,- only"
@input="channel.id = $event.target.value" />
<f7-list-item v-else :disabled="disabled" media-item class="channel-item" title="Channel UID">
<div slot="subtitle">
<f7-list-input v-else label="Channel UID" type="text" :input="false" disabled>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was set to list-item instead of list-input because channel UIDs tend to be long

Copy link
Contributor

Choose a reason for hiding this comment

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

It now looks like this:
Desktop:
image

iPhone:
image

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I will revert to the old look here, but keep the transformation UID the new way (those usually aren't too long).

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05 florian-h05 merged commit a5869f3 into openhab:main Feb 18, 2024
6 checks passed
@jimtng jimtng deleted the uniform-entity-id-labelling branch February 18, 2024 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request main ui Main UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change Name to <object> ID in all edit pages
5 participants