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

Flattening "committees" page components #170

Merged
merged 2 commits into from
Jun 15, 2021

Conversation

mattxwang
Copy link
Contributor

Hello there! This is a convenience PR to knock out some refactoring (#131) before we make a big site migration (which would involve changing a bunch of components). Basically, this PR does three things:

  • moves tiny components with no state into their parent component, to reduce nesting; involves some renaming
  • collates styles together
  • adjusts some CSS to make the above two changes no-ops on visuals

Overall, the impact on the end user should be nothing - this PR just aims to improve the developer experience. I've tested locally on my end side-by-side and I see no issues, but a second look would be great!

Two quick notes:

  • left some TODOs on weird JS things I saw
  • our CSS has way too much overlap - hopefully this is something we can resolve with Gatsby/Next, as we can build custom bundles for each page. we may also want to rethink how a lot of our current CSS is written, there's way too much namespace overlap and specificity mixing

@mattxwang mattxwang requested a review from advaithg June 14, 2021 18:33
Copy link
Contributor

@advaithg advaithg left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Deploy preview works fine
Comments make sense to me - they're things we should do when working on the transition

Major changes are all in removing unnecessary files with minimal code imported only in one place and putting the code in the places where it's imported
The most obvious changes are in the Committee Sections, with more changes in the banner, sidebar and navbar
Styling is also more centralized now

Probably can still make it cleaner if possible, considering the repeated CSS and still somewhat convoluted paths in some places (like src/components/Committees/CommitteeSections/CommitteeSectionIntro/CommitteeSectionIntro.js)

@mattxwang mattxwang merged commit a510659 into master Jun 15, 2021
@mattxwang mattxwang deleted the flatten-committees-components branch June 15, 2021 08:19
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