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

Moving a named element produces a syntax error #5195

Closed
tronical opened this issue May 7, 2024 · 3 comments · Fixed by #5196
Closed

Moving a named element produces a syntax error #5195

tronical opened this issue May 7, 2024 · 3 comments · Fixed by #5196
Labels
a:live preview The Live preview of slintpad or the LSP (mT,bO) bug Something isn't working

Comments

@tronical
Copy link
Member

tronical commented May 7, 2024

Steps to reproduce:

  1. Open https://snapshots.slint.dev/master/editor/
  2. Give the Text element in the hello world a name:
import { AboutSlint, Button, VerticalBox } from "std-widgets.slint";
export component Demo {
    VerticalBox {
        alignment: start;
        new-name-given := Text {
            text: "Hello World!";
            font-size: 24px;
            horizontal-alignment: center;
        }
        AboutSlint {
            preferred-height: 150px;
        }
        HorizontalLayout { alignment: center; Button { text: "OK!"; } }
    }
}
  1. Select "Design Mode" in the live-preview's toolbar.
  2. Drag the "Hello World!" text label in the live-preview and drop it further down in one of the drop areas marked with a green line.

Observe how the Text { ... } element is moved, but the new-name-given := part is left in the original place, resulting in a syntax error.

@hunger
Copy link
Member

hunger commented May 7, 2024

Moving that element inside the same parent can be done with Olivier's PR. It will usually fail when you move an element with an ID into some other parent. The id is used somewhere, and tht turns into an syntax error when the id is gone.

The best approach is probably to now allow moving anything with an id. Or at least not allow moving it into a new parent maybe?

Or maybe we could copy the Element with an id instead of moving it?

I wonder how to make it obvious what happens when we get into those special modes of copying/moving based on context.

@ogoffart
Copy link
Member

ogoffart commented May 7, 2024

Moving/removing an element with an id shouldn't be a problem. The problem is moving elements into another scope. if either the element itself uses it, or it is used and ends up in another scope.

So it is a problem if

  • the moved element has bindings that references parent (these could be rewritten)
  • the moved element has bindings that references something in its old scope which is no longer in the new scope (this operation should be disallowed?)
  • Some other element reference the moved element which is moved in another scope (this operation should be disallowed?)

When disallowing, a message should explain why.

@tronical
Copy link
Member Author

tronical commented May 7, 2024

When disallowing, a message should explain why.

It would be nice to visualise the area of the scope. So a repeated element itself isn't movable, a child (or grand-child) of the repeated element should be movable within the area of the scope element's area. Everything outside could perhaps be visually masked (like grey'ed out), but definitely not accepted as drop location.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:live preview The Live preview of slintpad or the LSP (mT,bO) bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants