[pull] develop from mermaid-js:develop - #227
Merged
Merged
Conversation
…slot
A named attribute lands in a positional slot when the argument before it is
omitted, and the handlers for the text slots (`label`, `descr`, `techn`,
`type`) wrapped whatever arrived in `{ text: value }` regardless of which
field it was actually for. So
System(s, "S", $tags="cylinder")
stored `tags` as `{ text: 'cylinder' }`, while
System(s, "S", "desc", $tags="cylinder")
stored the string. Consumers read `$tags` as a string - `resolveNodeShape`
calls `.split(',')` on it - so the first form threw
`shape.tags.split is not a function` and the diagram failed to render.
Only a text field belongs in a `{ text }` wrapper, so the eleven slot
handlers now consult one set of field names and assign anything else raw,
matching what `assignAttributes` already did for the attributes that arrive
in their own slots.
Found while building `AddElementTag` support: the existing e2e case passes
only because its fixture supplies a description before `$tags`, so the
broken form was never exercised.
Regression tests cover both forms of `$tags`, a named `$descr` (which must
still be wrapped), `$sprite`/`$link`, and a relationship `$tags`; three of
the five fail without this change.
Addresses the review point on the previous commit, which turned out to
describe a live bug rather than only a test gap.
`Container` takes `techn` before `descr`, so in
Container(c, "C", $descr="a description")
the named attribute lands in the `techn` slot. That handler assigns
`descr` correctly - and then the `descr` handler runs, finds its own
argument absent, and overwrites the value with `{ text: '' }`. The
description was silently lost, not merely stored in the wrong shape, which
is why the previous commit's tests did not catch it: they exercise `System`,
where `descr` is the first optional slot and nothing follows it.
The "argument absent" branches now only fill in a default when the field has
not already been set, so an earlier handler's assignment survives. The same
applies to `techn` and `type`, and to relationships, where `techn` likewise
precedes `descr`.
Three tests added: a named `$descr` landing in an earlier slot on a
container and on a relationship, and a check that `descr` still defaults to
empty when it is genuinely absent. The first two fail without this change.
Completes the pair of fixes above by making `assignAttributes` follow the
same rule as the positional slot handlers.
`System_Boundary` and `Container_Boundary` splice their kind in as a
positional argument, so an explicit `$type` shifts one slot along and
arrives as a named attribute in the `tags` slot. `assignAttributes`
assigned every named attribute raw, so it overwrote the `{ text: 'SYSTEM' }`
the type slot had just set with the bare string `'V'` - and every consumer
reads `boundary.type.text`, so the type silently vanished. `Node` was
unaffected, because its type is a real positional slot.
Found by enumerating the combinations rather than by chance: this is the
third defect in this area, and each one only appeared for one particular
declaration kind and attribute. The tests now walk every kind of
declaration against every attribute that can reach its first optional slot -
34 combinations - and assert the stored shape. The two boundary `$type`
cases fail without this change.
fix(c4): stop wrapping non-text named attributes that land in a text slot
The canonical C4Context example doubles as an UpdateElementStyle demo: it restyles `customerA` red, and colours the relationships touching it, without saying so anywhere nearby. Read as a picture of the defaults, it looks like one Person renders differently from the others for no reason, which is how it was reported in #7491. A note under each of the two copies of that example, rather than a change to the example itself, so the demonstration stays where it is.
docs(c4): say that the example's red styling is a demonstration
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )