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

feat(dynamic-plugins): replace static MUI Icons library import with dynamic appIcons #736

Merged
merged 2 commits into from
Nov 10, 2023

Conversation

tumido
Copy link
Member

@tumido tumido commented Nov 8, 2023

Description

Instead of embedding all MUI4 icons into the bundle, we can depend in app.getSystemIcon() icon catalog. This PR allows users to extend it via dynamic plugins and therefore cleanup our bundle of unnecessary icons.

Which issue(s) does this PR fix

Depends on: janus-idp/backstage-plugins#919
Resolves: #725

PR acceptance criteria

Please make sure that the following steps are complete:

  • GitHub Actions are completed and successful
  • Unit Tests are updated and passing
  • E2E Tests are updated and passing
  • Documentation is updated if necessary (requirement for new features)
  • Add a screenshot if the change is UX/UI related

How to test changes / Special notes to the reviewer

@tumido tumido requested a review from a team as a code owner November 8, 2023 12:32
Copy link

changeset-bot bot commented Nov 8, 2023

🦋 Changeset detected

Latest commit: 361aa8e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
app Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented Nov 8, 2023

The image is available at: quay.io/janus-idp/backstage-showcase:pr-736!

Copy link
Contributor

github-actions bot commented Nov 8, 2023

The image is available at: quay.io/janus-idp/backstage-showcase:pr-736!

@tumido
Copy link
Member Author

tumido commented Nov 8, 2023

Hold for ocm plugin to release, we need to update it here. https://github.com/janus-idp/backstage-plugins/actions/runs/6801215513/job/18491731589

…ynamic appIcons

Signed-off-by: Tomas Coufal <tcoufal@redhat.com>
@tumido tumido added the cherry-pick-1.0.x This PR should be cherry-picked to the 1.0.x branch label Nov 8, 2023
Copy link
Contributor

github-actions bot commented Nov 8, 2023

The image is available at: quay.io/janus-idp/backstage-showcase:pr-736!

Copy link
Member

@schultzp2020 schultzp2020 left a comment

Choose a reason for hiding this comment

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

LGM

@gashcrumb
Copy link
Member

Of course after I approved I realized I was on the wrong branch. Should this icon no longer be showing up with this change in place:

image

@gashcrumb
Copy link
Member

Ah, sorry for the noise that's behaving as expected, I of course had it mis-configured locally :-)

Copy link

sonarcloud bot commented Nov 10, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Contributor

The image is available at: quay.io/janus-idp/backstage-showcase:pr-736!

@tumido tumido merged commit 79a7160 into janus-idp:main Nov 10, 2023
6 checks passed
@tumido tumido deleted the make-icons-dynamic branch November 10, 2023 23:51
@kadel kadel added the cherry-pick-OK This PR was successfully cherry-picked to the appropriate branch. label Nov 13, 2023
kadel added a commit to kadel/backstage-showcase that referenced this pull request Nov 13, 2023
…ynamic appIcons (janus-idp#736)

Signed-off-by: Tomas Coufal <tcoufal@redhat.com>
Co-authored-by: Tomas Kral <tkral@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-1.0.x This PR should be cherry-picked to the 1.0.x branch cherry-pick-OK This PR was successfully cherry-picked to the appropriate branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Menu icons from dynamic plugins should use imports
4 participants