Skip to content

Conversation

@nimec01
Copy link
Collaborator

@nimec01 nimec01 commented Jan 28, 2026

Closes #229

@nimec01
Copy link
Collaborator Author

nimec01 commented Jan 28, 2026

Funktioniert withSvgHighlighting = False aktuell wirklich, wie es in #229 (comment) vermutet wird?

Also führt das Ausschalten auch tatsächlich dazu, dass dies nicht mehr möglich ist?

Wenn ich diese Änderungen hier nämlich teste, führt dies bei mir nicht dazu, dass beim Hovern kein Highlighten mehr stattfindet. Beim Erstellen der SVGs wird aber der korrekte Wert withSvgHighlighting = False übergeben. Wie Marcellus in einem anderen Kommentar schon angemerkt hatte, ist das SVG Highlighting generell eingeschaltet. Um dies auszuschalten, dürften im SVG, meinem Verständnis nach, keine Klassen wie "node", "edge", ... zugewiesen werden. Die Klassen werden aber anscheinend trotz withSvgHighlighting = False immernoch hinzugefügt.

@jvoigtlaender
Copy link
Member

Well this comment: #260 (comment) stated that yes, stuff is still annotated when this setting is used, but the annotations are somehow problematically structured then (for reasons beneficial elsewhere) and that

destroys the structure for highlighting

So, the highlighting should be effectively (though "accidentally") turned off, then.

I guess the first thing to check now is whether flipping that config bit makes any difference for the generated svg at all. (It should have.)

@jvoigtlaender
Copy link
Member

Presumably the changes to src/Modelling/PetriNet/Diagram.hs in e2d0933 are what "destroyed the structure".

That is, the svgClass $ ' ' : l in the last line here:

additionalLabel
| withSvgHighlighting = id
| otherwise = svgClass $ ' ' : l

is what should be different/relevant between svgs intended for highlighting and svgs for which highlighting would/should not work.

@nimec01
Copy link
Collaborator Author

nimec01 commented Jan 28, 2026

I guess the first thing to check now is whether flipping that config bit makes any difference for the generated svg at all. (It should have.)

The generated SVGs seem to be different. There are additional classes for the elements in the "without highlight" SVG (like class="rect" vs class="rect t4). But this does not make any difference for me in terms of highlighting.

[Interestingly, the class names also differ for me when I generate them through Autotool or manually in ghci (class="rect t4 vs class="rect Transition 4). But that does also not make any difference.]

@nimec01
Copy link
Collaborator Author

nimec01 commented Jan 28, 2026

Could it be that the “hack” no longer works? I generally mislike the idea to implement features by abusing some kind of "bug".

It probably shouldn't be too hard to just add a class to the (root) SVG element in order to disable highlighting (also updating the CSS rules accordingly).

Also: attaching arbitrary data to any HTML (or SVG) element is pretty easy. You can just use data-* attributes. Implementing label annotations in this way should not affect the rendering/highlighting of the SVG at all.

That way it would be easier to separate the two concerns.

@jvoigtlaender
Copy link
Member

Okay. I think the best way forward, then, is:

  • Make the additionalLabel printing in drawNode not dependent on the withSvgHighlighting parameter, but instead on an additional parameter withoutAdditionalLabels of the drawNode/drawGraph functions which is always set to True in this codebase.
  • Use the withSvgHighlighting parameter in drawNode to instead turn on/off the svgClass-annotating overall.
  • Then proceed in the spirit of Adaption Hovering bei Aufgaben mit Petrinetzanzeige #229.

@jvoigtlaender
Copy link
Member

(That was written without seeing your comment. Maybe your approach is ultimately the conceptually better one. But there are other constraints here, like what the additional labels were originally meant for, and also a desire to not otherwise change the css now. I'd opt for the minimal immediate change that will have the desired impact.)

@nimec01
Copy link
Collaborator Author

nimec01 commented Feb 2, 2026

It seems like my concept isn't that easy to implement here. I will elaborate more in #562.

@jvoigtlaender
Copy link
Member

You introduced a new parameter into the lecturer-facing config data types. That is not what I had in mind above:

  • Make the additionalLabel printing in drawNode not dependent on the withSvgHighlighting parameter, but instead on an additional parameter withoutAdditionalLabels of the drawNode/drawGraph functions which is always set to True in this codebase.

That was purely about having an additional parameter in the drawNode/drawGraph functions, and setting that parameter always (not only when the lecturer said or did not say something in the config) to True in this codebase.

(Someone outside this codebase, but using it as a library, might call these functions with that parameter set to False, though. That's the design I had in mind.)

@nimec01
Copy link
Collaborator Author

nimec01 commented Feb 10, 2026

You introduced a new parameter into the lecturer-facing config data types. That is not what I had in mind above:

  • Make the additionalLabel printing in drawNode not dependent on the withSvgHighlighting parameter, but instead on an additional parameter withoutAdditionalLabels of the drawNode/drawGraph functions which is always set to True in this codebase.

That was purely about having an additional parameter in the drawNode/drawGraph functions, and setting that parameter always (not only when the lecturer said or did not say something in the config) to True in this codebase.

(Someone outside this codebase, but using it as a library, might call these functions with that parameter set to False, though. That's the design I had in mind.)

Got it. However, always setting that variable to True i.e. enforcing label annotations wouldn't allow us to use SVG highlighting anymore (as described here and here)

@jvoigtlaender
Copy link
Member

How does setting a parameter withoutAdditionalLabels to True amount to "enforcing label annotations"? 😄

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.

Adaption Hovering bei Aufgaben mit Petrinetzanzeige

2 participants