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

frontend: Abstract AppLogo with plugins' logic into its own component #1125

Merged
merged 1 commit into from
Jun 6, 2023

Conversation

joaquimrocha
Copy link
Collaborator

@joaquimrocha joaquimrocha commented May 5, 2023

The AppLogo logic was being used as a very simple component just
displaying the original logo and thus was used in two different places
with the logic for checking for a plugin overridden logo and error
boundary, etc.
These changes abstract all that into a component that can just be
added anywhere that it's needed.
The file is also moved from Sidebar/AppLogo to App/AppLogo, as it's a
more logical place for this component (the component is also in the
chooser, not just in the sidebar).

(This abstraction will be useful for when we implement the new design where the logo may be moved elsewhere)

How to test:

  • Verify the logo is correctly displayed in both the sidebar and the chooser
  • Do the same when the sidebar is collapsed
  • Do the same but after running the change-logo example plugin

@joaquimrocha joaquimrocha requested a review from illume May 5, 2023 09:51
@joaquimrocha joaquimrocha added the frontend Issues related to the frontend label May 5, 2023
@joaquimrocha joaquimrocha added this to the v0.18.0 milestone May 5, 2023
Copy link
Collaborator

@illume illume left a comment

Choose a reason for hiding this comment

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

👍 nice improvement. Thanks.

@joaquimrocha joaquimrocha force-pushed the abstract-app-logo branch 2 times, most recently from 979f486 to 5fabc9a Compare June 6, 2023 16:45
The AppLogo logic was being used as a very simple component just
displaying the original logo and thus was used in two different places
with the logic for checking for a plugin overridden logo and error
boundary, etc.
These changes abstract all that into a component that can just be
added anywhere that it's needed.
The file is also moved from Sidebar/AppLogo to App/AppLogo, as it's a
more logical place for this component (the component is also in the
chooser, not just in the sidebar).
@joaquimrocha joaquimrocha merged commit 890f19c into main Jun 6, 2023
@joaquimrocha joaquimrocha deleted the abstract-app-logo branch June 6, 2023 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend Issues related to the frontend
Projects
Development

Successfully merging this pull request may close these issues.

2 participants