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.

Sign up for GitHub

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

Jump to bottom

Paint by county with ID instead of features #199

Merged
merged 16 commits into from
Nov 21, 2024
Merged

Paint by county with ID instead of features #199

merged 16 commits into from
Nov 21, 2024

Conversation

nofurtherinformation
Copy link
Collaborator

Instead of painting by county using features, this PRs drafts a method of caching all IDs and then filtering based on those

Description

  • Cache IDs of features on data in the map
  • When painting by county, search those IDs

Reviewers

@nofurtherinformation
Copy link
Collaborator Author

Also filters counties to only active state

@nofurtherinformation nofurtherinformation marked this pull request as ready for review November 21, 2024 13:46
raphaellaude
raphaellaude requested changes Nov 21, 2024
View reviewed changes
Copy link
Collaborator

@raphaellaude raphaellaude left a comment

Choose a reason for hiding this comment

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

Sort of rushed through this review so apologies it's not more in depth. Only one comment re: unavailable county layers breaking things

app/src/app/store/mapRenderSubs.ts Outdated Show resolved Hide resolved
app/src/app/utils/helpers.ts Show resolved Hide resolved
@nofurtherinformation nofurtherinformation self-assigned this Nov 21, 2024
@nofurtherinformation nofurtherinformation added bug Something isn't working frontend labels Nov 21, 2024
raphaellaude
raphaellaude requested changes Nov 21, 2024
View reviewed changes
Copy link
Collaborator

@raphaellaude raphaellaude left a comment

Choose a reason for hiding this comment

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

almost there!

app/src/app/store/mapRenderSubs.ts Show resolved Hide resolved
raphaellaude
raphaellaude approved these changes Nov 21, 2024
View reviewed changes
Copy link
Collaborator

@raphaellaude raphaellaude left a comment

Choose a reason for hiding this comment

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

Lgtm

@nofurtherinformation nofurtherinformation merged commit d2ceb72 into main Nov 21, 2024
1 check passed
@nofurtherinformation nofurtherinformation deleted the paint-by-county-improvements branch November 21, 2024 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raphaellaude raphaellaude raphaellaude approved these changes

Assignees

@nofurtherinformation nofurtherinformation

Labels
bug Something isn't working frontend
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nofurtherinformation @raphaellaude