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

constant near keys should be able to be set on containers #848

Open
alixander opened this issue Feb 19, 2023 · 7 comments
Open

constant near keys should be able to be set on containers #848

alixander opened this issue Feb 19, 2023 · 7 comments
Labels

Comments

@alixander
Copy link
Collaborator

alixander commented Feb 19, 2023

as long as nothing within is connected to anything outside

direction: right

x -> y: {
  style.stroke: green
}

y -> z: {
  style.stroke: red
}

legend: {
  # This is the TODO
  near: bottom-center
  color1: foo {
    shape: text
    style.font-color: green
  }

  color2: bar {
    shape: text
    style.font-color: red
  }
}
@alixander
Copy link
Collaborator Author

@donglixiaoche if you want to try a hard one!

@ShupingHe
Copy link
Contributor

@alixander yep,a hard one,exactly what I want

@alixander
Copy link
Collaborator Author

@donglixiaoche
so, for context, you can set near to constant values on individual shapes. it's implemented here: #525

but people commonly want to make a legend, which is a container of stuff, and put it in the top-center.

setting constant nears works roughly like:

  1. identify which objects are constant nears
  2. remove them before the main layout engine runs
  3. run main layout engine
  4. add them back in to the appropriate spot

for containers, i think you'll have to

  1. validate in the d2compiler that nothing in the container or descendant of container is connected to anything outside, (because if it is, it has to run in the main layout engine)
  2. remove that container and all descendants of that container, including connections between children of that container
  3. add all those things back in after main layout engine runs

@ShupingHe
Copy link
Contributor

@alixander just so you know, i'm still working on this issue...

as you mentioned, for shapes with near keyword, before main layout, i need to remove that container and all descendants of that container.
but if having descendants is allowed for container with near keyword, these descandants also need to be layouted, which can't be done if we remove them from the graph.
so i was thinking reolve this problem by contrusting a new graph, and then run another main layout? am i correct? or do you have any ideas?

@alixander
Copy link
Collaborator Author

@donglixiaoche oh that's a good point, i didn't think about that.

  1. It looks like the core layout is coupled with sequence diagram layouts. that should probably be separated:

    d2/d2lib/d2.go

    Line 73 in 7ba463c

    err = d2sequence.Layout(ctx, g, coreLayout)
  2. The descendants would also need to undergo layout, but they themselves might also have near containers, or sequence diagrams. So it should be recursive. Which means this whole portion of layout code should probably be separated so that it can be run recursively:

    d2/d2lib/d2.go

    Lines 66 to 81 in 7ba463c

    coreLayout, err := getLayout(opts)
    if err != nil {
    return nil, err
    }
    constantNears := d2near.WithoutConstantNears(ctx, g)
    err = d2sequence.Layout(ctx, g, coreLayout)
    if err != nil {
    return nil, err
    }
    err = d2near.Layout(ctx, g, constantNears)
    if err != nil {
    return nil, err
    }
    . By doing this, perhaps (1) won't be necessary.

contrusting a new graph, and then run another main layout?

yes, each constant-near-container would have to be its own graph. so you remove all the constant-near-containers, run layout on all of them (recursively), then run on the main graph, then re-insert all the objects and edges back into the main graph.

quite complicated, let me know when you run into anything at all, always available to help

@ShupingHe
Copy link
Contributor

@alixander i didn't think about the recursive way cause previously only root level shape could have a near attribute, ok then, let me think about it

@alixander
Copy link
Collaborator Author

@donglixiaoche yeah wait actually that is the case right now. sorry got confused with this issue: #841

in that case, no need to handle non-root nears. they're still illegal for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

No branches or pull requests

2 participants