Skip to content

Move logo to components folder. #985

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

Closed
wants to merge 1 commit into from

Conversation

saurabhp75
Copy link
Contributor

Move Logo component to components folder.

Test Plan

  • Run unit tests and e2e test to ensure there is no regression.

Checklist

NA

Screenshots

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Why is this necessary?

@saurabhp75
Copy link
Contributor Author

Logo component looks out of place in root route, it should rather be in components folder where other pages can also import if required. The PR follows "full stack components" approach.

@kentcdodds
Copy link
Member

Thanks, but personally I don't think it's necessary to move it unless you really do need to use this component elsewhere. Thanks anyway!

@kentcdodds kentcdodds closed this Apr 6, 2025
@saurabhp75
Copy link
Contributor Author

saurabhp75 commented Apr 6, 2025

Edit: Gives more flexibility to user in implementing the Logo and it's use anywhere. For eg. the user can add a "size" prop and use the logo in a form elsewhere.

Current implementation assumes that logo shall be used only in root route, which imho is restricting.

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.

2 participants