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

Remove query and site props passing from everywhere except class components #4340

Merged
merged 2 commits into from
Jul 12, 2024

Conversation

apata
Copy link
Contributor

@apata apata commented Jul 11, 2024

Changes

Followup to #4334, to further alleviate issues from props passing

Tests

  • Automated tests have been added
  • This PR does not require tests

Changelog

  • Entry has been added to changelog
  • This PR does not make a user-facing change

Documentation

  • Docs have been updated
  • This change does not need a documentation update

Dark mode

  • The UI has been tested both in dark and light mode
  • This PR does not change the UI

@apata apata force-pushed the alleviate-props-passing-further branch from 2f4a3f5 to 5fffacc Compare July 11, 2024 10:28
@apata apata requested a review from RobertJoonas July 11, 2024 11:21
function MatchDayOfWeekInput({ history, query, site }) {
function MatchDayOfWeekInput({ history }) {
const site = useSiteContext()
const { query } = useQueryContext()
Copy link
Contributor

Choose a reason for hiding this comment

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

Would something like useDashboardContext() make sense which would encapsulate both site AND query? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

I considered it initially, but my hunch is that many of these components don't really need either.

I think the way forward is to actually remove some of these superfluous dependencies, and provide components with specific handlers for what they're supposed to do: in this example handleCompareExactMatch, handleCompareDayOfTheWeek.

Such handlers would be declared higher up in the React tree and themselves dependent on a shared API component, which is dependent on the site.domain.

@apata apata added the preview label Jul 11, 2024
Copy link

Preview environment👷🏼‍♀️🏗️
PR-4340

@apata apata force-pushed the alleviate-props-passing-further branch from 336a058 to 3195912 Compare July 11, 2024 14:07
@apata apata added deploy-to-staging Special label that triggers a deploy to a staging environment preview and removed preview deploy-to-staging Special label that triggers a deploy to a staging environment labels Jul 11, 2024
Copy link

Preview environment👷🏼‍♀️🏗️
PR-4340

1 similar comment
Copy link

Preview environment👷🏼‍♀️🏗️
PR-4340

@apata apata added preview and removed preview labels Jul 11, 2024
Copy link

Preview environment👷🏼‍♀️🏗️
PR-4340

@apata apata force-pushed the alleviate-props-passing-further branch from 3195912 to f41120f Compare July 12, 2024 07:40
Copy link
Contributor

@macobo macobo left a comment

Choose a reason for hiding this comment

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

Didn't test it manually but the approach and code looks great! 🚢

@apata apata merged commit 79188a9 into master Jul 12, 2024
10 checks passed
@apata apata deleted the alleviate-props-passing-further branch July 12, 2024 07:59
RobertJoonas pushed a commit that referenced this pull request Jul 12, 2024
…onents (#4340)

* Remove query and site props passing from everywhere except class components

* Fix invalid import
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants