Skip to content

disallow edit frames if component comes from fragment with varibles d…#715

Merged
mattmacf98 merged 7 commits into
mainfrom
FEAT-disable-edit-frame-for-variable-fragments
May 27, 2026
Merged

disallow edit frames if component comes from fragment with varibles d…#715
mattmacf98 merged 7 commits into
mainfrom
FEAT-disable-edit-frame-for-variable-fragments

Conversation

@mattmacf98
Copy link
Copy Markdown
Member

@mattmacf98 mattmacf98 commented May 27, 2026

Details

In this ticket https://viam.atlassian.net/browse/APP-15772 we saw people had issues editing frames coming from fragments that use variables overwriting the variables. This is technically correct (fragment mods should override variables)

{
  "fragments": [
    {
      "id": "a8c0baf1-8e14-4bae-aa74-3c98b172262a",
      "variables": {
        "translation_x": 1000
      }
    }
  ],
  "fragment_mods": [
    {
      "fragment_id": "a8c0baf1-8e14-4bae-aa74-3c98b172262a",
      "mods": [
        {
          "$set": {
            "components.base-1.frame": {
              "translation": {
                "x": 100,
                "y": 0,
                "z": 0
              },
              "parent": "world",
              "orientation": {
                "type": "ov_degrees",
                "value": {
                  "x": 0,
                  "y": 0,
                  "z": 1,
                  "th": 0
                }
              },
              "geometry": {
                "type": "box",
                "x": 100,
                "y": 100,
                "z": 100
              }
            }
          }
        }
      ]
    }
  ]
}

this was a little confusing to users who didn't understand they were overwriting variables so we will disable editing components from fragments which have variables

Testing

  • ran locally, saw I can still edit components from fragments with no variables and saw that I can no longer edit when they have variables
demo-disable-fragment-variable.mov
Screenshot 2026-05-27 at 3 05 25 PM

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 27, 2026

🦋 Changeset detected

Latest commit: d231c9d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@viamrobotics/motion-tools Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 27, 2026

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-05-27 19:37 UTC

Comment thread src/lib/hooks/usePartConfig.svelte.ts Outdated
current: Struct
isDirty: boolean
componentToFragId: Record<string, string>
componentToFragInfo: Record<string, FragmentInfo>
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this will require a change when we bump motion-tools in app to change the param type

@mattmacf98
Copy link
Copy Markdown
Member Author

I am also fine trying to do this after we move edit frames to a plugin if it makes it too messy to extract out with this new logic in to CC @micheal-parks

const isFrameNode = $derived(!!framesAPI.current)
const isGeometry = $derived(!!geometriesAPI.current)
const showEditFrameOptions = $derived(isFrameNode && partConfig.hasEditPermissions)
const isFragmentComponentWithVariables = $derived(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe we should add a small message in the details panels saying something along the lines of "This frame is not editable due to the presence of fragment variables"?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

oh yeah I'll add something for that

@micheal-parks
Copy link
Copy Markdown
Member

All good with merging this first before plugin making

@mattmacf98 mattmacf98 merged commit a62cf24 into main May 27, 2026
7 checks passed
@mattmacf98 mattmacf98 deleted the FEAT-disable-edit-frame-for-variable-fragments branch May 27, 2026 19:37
@claude claude Bot mentioned this pull request May 27, 2026
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.

3 participants