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

Incorporate classnames package and do some general cleanup #40

Merged
merged 9 commits into from
Oct 24, 2024

Conversation

jakewheeler
Copy link
Collaborator

@jakewheeler jakewheeler commented Oct 24, 2024

Summary

This PR does the following:

  • Adds and makes use of classNames where appropriate
  • Adds some reasonable defaults for prettier and eslint
    • A couple of new devDependencies were added for this
  • Formats all applicable files using the new prettier defaults
  • Ensures the Footer component renders a footer element rather than another header
  • Moves NavigationLink and its prop type into its own file
  • Moved the heavily used basePath variable into its own constants.ts file
  • Fixed an error where Next.js couldn't find the favicon.ico
  • Some other minor cleanup

Testing

  1. Run npm i to install the new packages
  2. Start the project (restart might be needed)
  3. Make sure everything generally still works and looks the same

If you happen to run into any issues, it might be worth trying to delete the .next folder and restarting your dev build.

Discussion

The "reasonable defaults" felt like a good starting point to me but we can definitely adjust these over time if we think any of the ESLint rules aren't useful to us.

@jakewheeler jakewheeler changed the title WIP - Incorporate classnames package and do some general cleanup Incorporate classnames package and do some general cleanup Oct 24, 2024
@jakewheeler jakewheeler marked this pull request as ready for review October 24, 2024 18:56
@jakewheeler jakewheeler merged commit 8e939d5 into next Oct 24, 2024
1 check passed
@nickbristow nickbristow deleted the jw/2811-classnames branch October 24, 2024 21:03
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