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

Refactor selections #3837

Open
1 of 3 tasks
Irev-Dev opened this issue Sep 8, 2024 · 1 comment
Open
1 of 3 tasks

Refactor selections #3837

Irev-Dev opened this issue Sep 8, 2024 · 1 comment

Comments

@Irev-Dev
Copy link
Collaborator

Irev-Dev commented Sep 8, 2024

Selections for the longest time has simply been a source-range with some metadata about what the selection is. this seemed like a neat idea at first but it make sense to use artifacts from our artifactGraph instead.

How selections should work and what the refactor should look like is covered in

https://docs.google.com/document/d/16EROjbvGZnBg7Y43yaA36mS4SkC8Cxpm2gpZ1mY2UJo/edit#heading=h.z6ie6kxdgixv

  • Create the types of the new types
  • Implement convertSelectionToOld and convertSelectionsToOld as utils to help with the refactor
  • Update Xstate context to use the new types, this will require all the places that consume the types to use the convertSelectionToOld etc utils, though consider converting some of them to use the new types directly if that's easier.

This is what we're aiming for
image

The "find artifactthat matches sourceRange and type" should be able to use the existing codeToIdSelections function.

The types for the first task has been setup already #3836 note that

export type Selections = {
  otherSelections: Axis[]
  graphSelections: Artifact[]
}

maybe should be

export type Selections = {
  otherSelections: Axis[]
  graphSelections: ArtifactId
}

Instead not sure if one is better than the other.

@Irev-Dev
Copy link
Collaborator Author

Irev-Dev commented Sep 8, 2024

From the diagram, don't take id too seriously, like in some of those arrows it might be Artifact that is passed around, so long as the id is on the Artifact we good.

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

No branches or pull requests

1 participant