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

Enhanced shattering #129

Merged
merged 108 commits into from
Nov 12, 2024
Merged

Enhanced shattering #129

merged 108 commits into from
Nov 12, 2024

Conversation

nofurtherinformation
Copy link
Collaborator

@nofurtherinformation nofurtherinformation commented Oct 17, 2024

This PR works to enhance the shattering UX to support the use cases and detailed work on shattered geographies.

Description

The workflow this aims to accomplish is:

  • A user selects a parent geography to shatter
  • They draw on the blocks that are children of the shattered geography. No other geographies are interacted with
  • The user exists the shatter view
  • The user can re-enter the shatter view
  • When outside of the shatter view, drawing over the shattered children will assign them as a parent geography

WIP screencap:

Screen.Recording.2024-10-17.at.12.25.18.PM.mov

Reviewers

  • Primary:
  • Secondary:

Checklist

  • Focus child geographies on shatter
  • Lock non-child geographies on shatter
  • Re-shatter
  • Save parent-child edges to browser on shatter
  • Save parent-child edges to browser on load
  • Handle hover, paint, and other interactions when drawing over a shattered parent
  • Locking function, probably
  • Refactor to better abstraction for going from ReturnType<queryRenderFeatures> -> various map actions There are some refactors that could be good for a future PR, but I think we should mostly leave it for now.
  • Test user interactions
  • Merge main and resolve conflicts
  • Translucency of unpaintable areas is too subtle
  • We should add a big outline for the paintable area / VTD being shattered to make it obvious where you can paint
  • If we paint the entire set of blocks one color OR don't paint at all, we should unshatter the VTD.
  • Rename to "shatter mode" rather than "shatter view"

Screenshots (if applicable):

@nofurtherinformation
Copy link
Collaborator Author

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.

Looking really good – two v small comments, only one blocking.

@@ -174,6 +175,33 @@ async def shatter_parent(
return result


@app.patch(
Copy link
Collaborator

Choose a reason for hiding this comment

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

H: Sorry forgot to mention this before but could you add a test for this function, too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, yes 100%. Had this on my list todo but totally forgot!

Comment on lines 147 to 153
'line-width': [
'case',
['boolean', ['feature-state', 'focused'], false],
10, // Width of 10 when focused
['boolean', ['feature-state', 'highlighted'], false],
10, // Width of 5 when highlighted
0, // Default width
Copy link
Collaborator

Choose a reason for hiding this comment

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

PP: What do you think of using a round cap? Looks a little messy with the super thick line. Also have a personal pref to reduce the thickness a bit cause it's really thick but won't die on that hill!

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Round cap is a good call!

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 6cc5e6e into main Nov 12, 2024
2 checks passed
@nofurtherinformation nofurtherinformation deleted the enhanced-shattering-rebase branch November 12, 2024 15:27
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