Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

updated the instance list icons #7207

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

k-vijay-05
Copy link

Fixes #6755

Before : (Hard to distinguish which are the locked or unlocked instances on the instance list)
Screenshot 2024-11-29 101923

After this PR : (easy to distinguish which are the locked or unlocked instances on the instance list)
Screenshot 2024-11-29 101608

@k-vijay-05 k-vijay-05 requested a review from 4ian as a code owner November 29, 2024 10:30
@k-vijay-05
Copy link
Author

@4ian please review the PR!!

@4ian
Copy link
Owner

4ian commented Dec 2, 2024

Please ask nicely (no need for these exclamation marks) and wait for a few more days before bumping a PR, especially when there is a week-end during this time.

As for the PR:

  • The icons are a good idea but they seem too big compared to the text.
  • Z order is different than the Z position, so I think the Z order column should not be shown just after X and Y, but after the layer column.
  • We might take the opportunity to replace X and Y by "position" and display them as "X;Y" or "X;Y;Z" instead, that would give more space for other columns?
  • The "not locked" icons look a disabled button, so it would be inconsistent with the rest of the UI (a disabled icon can't be clicked)... I think we should brainstorm other ways to do it.

@LuniMoon
Copy link
Collaborator

LuniMoon commented Dec 2, 2024

This is an implementation from #6755 (comment) and I didn't specify spacing nor color. Sorry for that
I've attached a pdf with all the specs (size, colour) made with an automation tool (this is why it might look "over specified"). Feel free to use it as a reference if needed:
Specs-Instance panel.pdf

This is the mock up version in the pdf file for the PR:

  • The icon sizes are now specified (16px). They cannot be smaller (the tool doesn't have smaller icons).
  • Z order has been moved to the right of the layer column.
  • I did not merge X and Y positions because the instance panel displays them as separate values, so I prefer to leave them like this for now (I don't know if beginners will understand this inconsistency).
  • The "not locked" icon now has the "secondary text" color style instead of the "disable text" color style.

This is the updated version:
Screenshot 2024-12-02 at 12 27 39

@k-vijay-05 k-vijay-05 requested a review from Bouh December 20, 2024 16:45
@Bouh Bouh requested a review from AlexandreSi December 22, 2024 22:18
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Layer panel icon components for better comprehension
4 participants