Always set every style data field so a stale value cannot be stranded - #2122
Closed
kmcginnes wants to merge 1 commit into
Closed
Always set every style data field so a stale value cannot be stranded#2122kmcginnes wants to merge 1 commit into
kmcginnes wants to merge 1 commit into
Conversation
`cy.json({ elements })` merges element data: `ele.data(obj)` adds and overwrites
keys but never deletes ones missing from the new object. Both optional fields
were therefore unclearable once applied — a node whose icon stopped resolving
(an uploaded icon that fails to load, retried three times and given up on) kept
rendering the previous icon while its colour updated around it, with no error
and no signal to the user that anything had failed.
`ge_iconUrl` now carries `"none"` when a type has no icon and `ge_lineDashPattern`
carries cytoscape's default for solid lines, so both live on the base `node` /
`edge` rule and always reflect current state. That also moves the icon gate out
of the generic `components/Graph` component, next to the producer that feeds it,
and renames `__iconUrl` to match its siblings.
Collaborator
Author
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.
Description
A node could keep displaying a stale icon forever, with no error.
cy.json({ elements })merges element data:ele.data(obj)adds and overwrites keys but never deletes ones missing from the new object. Both of the optional style data fields were therefore unclearable once applied. Concrete path: a user replaces a type's icon, the new icon fails to load,iconRegistryretries three times and gives up, the type drops out of the background-image map,__iconUrlgoes absent — and the node silently keeps rendering the previous icon while its colour updates around it. The user gets no signal the upload failed and reasonably concludes it worked.This is a regression from the
data()migration in #2112, not preserved behaviour: the old stylesheet path setbackground-imageunconditionally, soundefinedcleared it.The fix is to make the fields total rather than to gate on presence.
ge_iconUrlcarries"none"(cytoscape's no-image value) when a type has no icon, andge_lineDashPatterncarries cytoscape's default[6, 3]for solid lines, which ignore it. Both then live on the basenode/edgerule and always reflect current state, so there are no gated selectors left — the stylesheet is two rules.Note that adding
background-image: noneto the base rule alone would not have fixed this: the gated selector keeps matching while the key is still present, and overrides it.Two side benefits: the icon gate moves out of the generic
components/Graphcomponent next to the producer that feeds it, and__iconUrlis renamedge_iconUrlto match its siblings.ge_lineDashPatternhad the same defect latently — harmless today because cytoscape ignores the pattern for solid lines, but the same shape.How to read
graphElementStyleData.ts— theALWAYS_SETrationale and the two now-total fields.useGraphStyles.ts— mappers moved onto the base rules, gated selector deleted.useManageStyles.ts— the hardcoded gate deleted from the generic component.