sverweij / mscgen_js

text => sequence charts
https://mscgen.js.org
GNU General Public License v3.0
206 stars 25 forks source link

Can't display any event alongside an alt text box in MsGenny #233

Closed hatsunearu closed 8 years ago

hatsunearu commented 8 years ago

I can't seem to get an event to display concurrently with an alt text box: Example 1:

a, b;

b box b: "Do mundane stuff",  
a alt a: "altbox" {
    a box a : "Do cool stuff";  
};

Example 2:

a, b;

a alt a: "altbox" {
    a box a : "Do cool stuff";  
},
b box b: "Do mundane stuff";

Both of these don't achieve the desired result. Example 1 shows the "Do mundane stuff" box, but breaks the altbox, and Example 2 doesn't show "Do mundane stuff" at all.

Edit: Accidentally pressed the save button. Filled out the rest of the comment as I intended to originally.

hatsunearu commented 8 years ago

I found a workaround (or maybe it's supposed to be like this)

a, b;

a alt a: "altbox" {
    a box a : "Do cool stuff",
    b box b: "Do mundane stuff";
};

This causes the mundane box and the alt box to touch borders, though.

sverweij commented 8 years ago

That's a bug alright. Two actually

  1. The renderer has no business hiding elements like that (sample 1 & 2)
  2. The touching borders hamper legibility

Thanks for the super clear minimal reproduction samples! They'll help a lot in identifying the root cause.

I'll try to set the renderer straight about this.

sverweij commented 8 years ago

@hatsunearu I've made a fix for the touching borders & temporarily deployed it in a feature branch - see issue #234 for links and before/ after renditions so your workaround will at least look the part. Let me know what you think

I'll try to have a look at #233 later this week.

hatsunearu commented 8 years ago

Looks like my 2nd example shows behavior I expect--but that branch doesn't fix the first example (I think this is what you're saying but just making sure). Thanks, it worked well for my original, more complicated diagram as well.

sverweij commented 8 years ago

@hatsunearu it's working now as originally intended. No events get eaten anymore, neither do any alt boxes. It might not look entirely like you'd expect it to, though:

after😬

When I looked my 2013 self in the eye I initially shook my head in mild disapproval. Until I realized this is a compromise. You can place something in parallel with the alt box but also in parallel with events within that alt box: moar-parallel

I have to ponder if this is still the best compromise, however. What do you think?

(I've already applied the patches to the npm packages, and deployed them to live as even this compromise is better than it was before)

hatsunearu commented 8 years ago

I mean, if you wanted the first box in B to be concurrent with "do cool stuff", I would just pad the spot "do mundane stuff" occupies with a pad symbol and it should work like that, if that's what you're asking.

Looks great though!