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

NAS-128844 / 24.10 / Use new icon to represent datasets #10058

Merged
merged 4 commits into from
May 15, 2024
Merged

Conversation

denysbutenko
Copy link
Member

Visit the Datasets page to see the changes. The original icon was renamed dataset_root, and a new dataset icon was added.

Demo:
image

@denysbutenko denysbutenko requested a review from a team as a code owner May 13, 2024 13:56
@denysbutenko denysbutenko requested review from bvasilenko and removed request for a team May 13, 2024 13:56
@bugclerk bugclerk changed the title NAS-128844: Use new icon to represent datasets NAS-128844 / 24.10 / Use new icon to represent datasets May 13, 2024
@bugclerk
Copy link
Contributor

Copy link

codecov bot commented May 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.85%. Comparing base (9c605a3) to head (c36b2b6).
Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10058      +/-   ##
==========================================
+ Coverage   73.80%   73.85%   +0.04%     
==========================================
  Files        1522     1520       -2     
  Lines       53252    53202      -50     
  Branches     6357     6352       -5     
==========================================
- Hits        39304    39290      -14     
+ Misses      13948    13912      -36     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@undsoft undsoft left a comment

Choose a reason for hiding this comment

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

  1. It's too large. It should be roughly the same size as zvol or dataset_root icon.
  2. It should be updated in ix-explorer.

Copy link
Contributor

@bvasilenko bvasilenko left a comment

Choose a reason for hiding this comment

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

Looks good!
New icon is displayed where it's expected to display

@denysbutenko
Copy link
Member Author

  1. It's too large. It should be roughly the same size as zvol or dataset_root icon.

Okay done
image

  1. It should be updated in ix-explorer.

ix-explorer uses three folder icons: folder, folder_open, and mdi-folder-lock. I've replaced folder icon for now.

image

@denysbutenko denysbutenko requested a review from undsoft May 14, 2024 04:00
@denysbutenko
Copy link
Member Author

Looking into reasons for test failing.

Copy link
Collaborator

@undsoft undsoft left a comment

Choose a reason for hiding this comment

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

New icon looks good.

@undsoft undsoft merged commit d097965 into master May 15, 2024
10 checks passed
@undsoft undsoft deleted the NAS-128844 branch May 15, 2024 09:35
@bugclerk
Copy link
Contributor

This PR has been merged and conversations have been locked.
If you would like to discuss more about this issue please use our forums or raise a Jira ticket.

@truenas truenas locked as resolved and limited conversation to collaborators May 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants