fix(diagram): keep trivial molecules visible while something is flagged - #170
Merged
Merged
Conversation
Curators: "H2O and H+ disappear with zooming out." Separately, #150: a molecule's chemical structure drawn with no molecule underneath it. Same fault from two sides -- the node was hidden while a different handler carried on drawing its structure. Flagging pins trivial molecules visible by marking them `.always-visible`, a class rather than an inline style precisely so nothing can lose it. But the zoom handler writes an *inline* opacity across every `.trivial` element, and in cytoscape an inline style beats any stylesheet rule, so it painted straight over the class. Detaching it from the zoom event -- what the previous fix did -- was not enough: `triggerZoom()` calls it directly, on every restyle and whenever an interactor opens. The class is now the authority: the handler leaves anything wearing it alone. Measured on beta, with a flag active, opacity is 1 at every zoom and across a restyle; with no flag, molecules still fade out and back as before. Also, the reason this took two attempts to land: `build:libs` did not build reactome-cytoscape-style. CI compensated with a step of its own, so a fix to that library passed CI and never reached the dev server, which builds the app without building the library. `build:libs` now covers every workspace library and `build` depends on it, so any path that produces the app produces what it links against -- and CI's two special-case steps are gone, because it now runs the same command a developer does. The regression test is end-to-end rather than a unit test: the library cannot be constructed headlessly (its layers plugin refuses), and the bug lives in what a real renderer computes. It reads opacity through the registration cytoscape keeps on its own container, which beats sampling canvas pixels -- those cannot say which element was faded. Verified to fail without the fix: both flagging tests go red, the unflagged one stays green. Fixes #150 Fixes #143 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Curators, testing on beta: "H2O and H+ disappear with zooming out." And separately #150: a molecule's chemical structure drawn with no molecule underneath it. Those are the same fault seen from two sides — the node was hidden while a different handler carried on drawing its structure.
Why the previous fix did not hold
Flagging pins trivial molecules visible by marking them
.always-visible— deliberately a class rather than an inline style, so nothing could lose it.But the zoom handler writes an inline opacity across every
.trivialelement, and in cytoscape an inline style beats any stylesheet rule. It painted straight over the class. Detaching it from the zoom event — what the previous attempt did — was not enough, becausetriggerZoom()calls it directly: on every restyle (a theme change, an analysis loading) and whenever an interactor is opened.The class is now the authority. The handler leaves anything wearing it alone.
Measured on beta, flag active:
With no flag, molecules still fade out as you zoom away and come back as you zoom in — unchanged.
Why it took two attempts to land
build:libsdid not buildreactome-cytoscape-style. CI compensated with a step of its own, so a fix to that library passed CI and never reached the dev server, which builds the app without building the library. I spent a while concluding the fix did not work when it had simply never been compiled.build:libsnow covers every workspace library andbuilddepends on it, so any path that produces the app produces what it links against. CI's two special-case steps are gone: it runs the same command a developer runs.The test
End-to-end rather than a unit test, for two reasons: the library cannot be constructed headlessly (its layers plugin refuses outright), and the bug lives in what a real renderer computes rather than in a pure function.
It reads opacity through the registration cytoscape keeps on its own container, which beats sampling canvas pixels — those cannot tell you which element was faded.
Verified to fail without the fix: both flagging tests go red, the unflagged one stays green.
Fixes #150
Fixes #143