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

WIP: Icons set #4054

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

WIP: Icons set #4054

wants to merge 5 commits into from

Conversation

fiskus
Copy link
Member

@fiskus fiskus commented Jul 22, 2024

Please, confirm that you are not against this idea, then I can finish the rest migration of the icons.

Description

  • Use "@material-ui/icons" instead of fonts fetched from CDN
  • Able to use outlined icons
  • Use correct and consistent icons for icons in file listing
  • WIP: Only expose those icons that we use
  • Rename icons: make naming based on the domain meaning instead of abstract MUI

Screenshot from 2024-07-22 18-40-08
Screenshot from 2024-07-22 18-40-17

TODO

  • Unit tests
  • Automated tests (e.g. Preflight)
  • Confirm that this change meets security best practices and does not violate the security model
  • Documentation
    • Python: Run build.py for new docstrings
    • JavaScript: basic explanation and screenshot of new features
    • Markdown somewhere in docs/**/*.md that explains the feature to end users (said .md files should be linked from SUMMARY.md so they appear on https://docs.quiltdata.com)
    • Markdown docs for developers
  • Changelog entry (skip if change is not significant to end users, e.g. docs only)

Copy link

codecov bot commented Jul 22, 2024

Codecov Report

Attention: Patch coverage is 0% with 33 lines in your changes missing coverage. Please review.

Project coverage is 37.68%. Comparing base (45070e6) to head (4d5d6cb).

Files with missing lines Patch % Lines
catalog/app/components/Icon.tsx 0.00% 20 Missing ⚠️
catalog/app/containers/Bucket/Listing.tsx 0.00% 4 Missing ⚠️
catalog/app/containers/Search/Sort.tsx 0.00% 2 Missing ⚠️
catalog/app/components/Intercom/Launcher.tsx 0.00% 1 Missing ⚠️
...ents/Preview/renderers/Perspective/Perspective.tsx 0.00% 1 Missing ⚠️
catalog/app/containers/Errors/FinalBoundary.tsx 0.00% 1 Missing ⚠️
catalog/app/containers/NavBar/BucketSelect.js 0.00% 1 Missing ⚠️
catalog/app/containers/NavBar/Controls.js 0.00% 1 Missing ⚠️
catalog/app/containers/NavBar/NavBar.tsx 0.00% 1 Missing ⚠️
catalog/app/containers/NavBar/NavMenu.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4054      +/-   ##
==========================================
- Coverage   37.72%   37.68%   -0.04%     
==========================================
  Files         766      767       +1     
  Lines       35148    35179      +31     
  Branches     5190     4999     -191     
==========================================
  Hits        13258    13258              
- Misses      21282    21313      +31     
  Partials      608      608              
Flag Coverage Δ
api-python 91.00% <ø> (ø)
catalog 12.10% <0.00%> (-0.02%) ⬇️
lambda 87.95% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@fiskus fiskus changed the title Icons set WIP: Icons set Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant