CODAP-1530: explain a legend attribute the display cannot honor

[kswenson]

Makes a display acknowledge a legend attribute it can't resolve per point, instead of accepting the drop and then going silent. Branches off leads-point-shapes; last piece of the LEADS Point Shapes epic.

Fixes CODAP-1530.

The state

When the legend attribute lives in a collection more childmost than the plotted cases, each point stands for several of its values at once, so no one color applies. Reach it either way round:

• Put Order on a graph's x-axis with Order as a parent collection, then drop Habitat on the plot.
• Assign a legend to a map, then drag lat/long out to a parent collection.

The points draw gray, and that is correct — it says the attribute was accepted but cannot be resolved per point. Everything else disagreed with it.

What changed

The legend explains itself. It renders the attribute label and a wrapped sentence where the keys would be, instead of returning null and taking the attribute's name and its remove action with it. MultiLegend includes such a layer rather than filtering it out, and the attribute menu takes the assigned ID as an override so the attribute is actually removable.

The palette stops offering controls that do nothing. No per-category rows, no numeric binning or range inputs, and no color control — every point is gray whatever it is set to. The display-wide shape control stays, because shape does still reach these points: getLegendShapeForCase falls back to the display's own shape while getLegendColorForCase overrides with the missing-value color. Its glyph draws in that color, so the palette shows what is plotted.

The map behaves like the graph. Three things were needed:

• getLegendColorForCase decides the inoperable case before the checks that read attributeID, since that is what differs between displays — the base configuration filters an unusable assignment out, GraphDataConfigurationModel's override keeps it. stylePoint follows, so a selection change does not repaint map points down the no-legend path.
• setLegendAttribute takes an attribute more childmost than a layer's position attributes instead of discarding it silently. Refusing never covered the case anyway, since moving a position attribute to a parent collection reaches the same state after the assignment is made. Layers that can honor the attribute rank ahead of those that cannot, with closeness choosing within a rank.
• A layer set to a heatmap it cannot draw shows points rather than nothing. The heatmap needs a legend attribute that resolves per case; the point gate required displayType to say "points", so between them the layer rendered neither.

Polygon layers deliberately keep their own color. A map carries one legend with no way to assign one per layer, so it reads as belonging to the points. Gray points on gray boundaries would say less than gray points on colored ones. The reasoning is recorded at the call site.

Scope, for a reviewer

This started as a legend-rendering change and grew. It now also changes what maps plot, which legend drops a map accepts, which layer a map assigns a legend to, and what a heatmap does when it cannot draw. The map changes are what it took to stop the map contradicting itself, but they are real behavior changes and worth reviewing as such.

Testing

Unit tests cover the model views, the word wrap, the palette in every legend type, the MultiLegend filter, and the map's layer selection — each checked by breaking the code and confirming the right test fails.

What is not covered: the React render and reaction paths, because no harness for them exists in these directories. That is legend.tsx choosing the message over null, the attribute-menu override, the heatmap fallback, and the visibility reaction that re-fires it. All four were verified by hand in the browser, on both a graph and a map, but a reviewer should know they rest on manual testing rather than on tests.

🤖 Generated with Claude Code

[cypress]

codap-v3    Run #12274

Run Properties:   Passed #12274  •  7a191080c2: Merge pull request #2701 from concord-consortium/CODAP-1530-inoperable-legend-at...

Project	codap-v3
Branch Review	leads-point-shapes
Run status	Passed #12274
Run duration	03m 21s
Commit	7a191080c2: Merge pull request #2701 from concord-consortium/CODAP-1530-inoperable-legend-at...
Committer	Kirk Swenson
View all properties for this run ↗︎

---

Test results
Failures	0
Flaky	0
Pending	0
Skipped	0
Passing	4
View all changes introduced in this branch ↗︎

[Copilot]

🟡 Changes recommended

Stale legend assignments can be misclassified and rendered as a blank inoperable legend.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds clear feedback when a graph cannot apply an assigned legend attribute to individual points.

Changes:

• Detects and explains inoperable legend assignments.
• Updates legend layout and palette behavior.
• Adds model, wrapping, and palette tests.

File summaries

File	Description
en-US.json5	Adds the explanatory message.
graph-data-configuration-model.test.ts	Tests inoperable legend detection.
data-configuration-model.ts	Exposes assigned legend state.
legend-color-controls.tsx	Hides unusable category controls.
legend-color-controls.test.tsx	Tests palette behavior.
legend.tsx	Renders the explanatory state.
legend.scss	Styles the message.
legend-attribute-label.tsx	Preserves the assigned attribute label.
inoperable-legend-message.tsx	Implements wrapping and sizing.
inoperable-legend-message.test.ts	Tests wrapping behavior.

Review details

• Files reviewed: 10/10 changed files
• Comments generated: 2
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov]

Codecov Report

❌ Patch coverage is 98.78049% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 88.02%. Comparing base (d7eee87) to head (1747755).

Files with missing lines	Patch %	Lines
v3/src/components/map/models/map-content-model.ts	83.33%	1 Missing ⚠️

Additional details and impacted files

@@                   Coverage Diff                   @@
##           leads-point-shapes    #2701       +/-   ##
=======================================================
+ Coverage               73.31%   88.02%   +14.70%     
=======================================================
  Files                     821      822        +1     
  Lines                   47595    47657       +62     
  Branches                12142    11717      -425     
=======================================================
+ Hits                    34893    41948     +7055     
+ Misses                  12687     5695     -6992     
+ Partials                   15       14        -1     

Flag	Coverage Δ
cypress	69.86% <94.20%> (+31.85%)	⬆️
jest	63.43% <63.41%> (+0.05%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

🔵 Needs a closer look

Numeric inoperable legends still expose ineffective controls and display a misleading gradient shape preview.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

v3/src/components/data-display/inspector/legend-color-controls.tsx:182

• For an inoperable numeric legend, hasFillGradient is still true, so fillGradientId overrides glyphColor and the shape preview remains multicolored instead of using missingColor. Suppress the gradient in the inoperable state so the glyph matches the plotted points.
  v3/src/components/data-display/inspector/legend-color-controls.tsx:194
• This return does not hide all ineffective color controls for a numeric inoperable legend. DisplayItemFormatControl still renders the binning selector and the flagged bin-count/range inputs whenever attributeType("legend") === "numeric", even though every plotted point is forced to missingColor; those controls therefore remain visible but cannot affect the display. Gate those sibling controls on legendAttributeIsInoperable as well, and cover the numeric state.

• Files reviewed: 10/10 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The shared operability flag incorrectly hides map color controls even though affected map points still use their display color.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 12/12 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The map parent filter prevents the new inoperable legend state from rendering for base configurations.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

v3/src/components/data-display/components/legend/legend.tsx:48

• The inoperable branch is unreachable for map layers. MultiLegend filters layers using dataConfiguration.attributeID('legend'), while the base configuration intentionally returns "" for this state, so a hierarchy change still removes the map legend and its remove action before Legend can render this message. Include visible layers whose legendAttributeIsInoperable is true (or whose assignedLegendAttributeID is set) in MultiLegend's filter.

• Files reviewed: 13/13 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[kswenson]

Fixed, and thanks for catching it — my previous reply on this PR was wrong. I wrote that the legend renders the message on maps too. It didn't: MultiLegend filters layers on attributeID('legend') at multi-legend.tsx:37, which the base configuration returns "" for in exactly this state, so the layer was dropped before Legend ran. I'd checked that MultiLegend renders the same Legend component and not the filter three lines above it.

The filter is now a named predicate, layerHasLegendToShow, which also admits a layer whose assigned attribute can't be honored. Five tests on it, including that a hidden layer is still skipped either way.

That makes the commit message on ffc4e54ed wrong as well — it claims maps already rendered the message. It's in history now; this comment is the correction.

[Copilot]

🟡 Changes recommended

Map legends can remain unremovable, select an inoperable layer over an operable one, or hide heatmap points entirely.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 19/19 changed files
• Comments generated: 3
• Review effort level: Balanced

[Copilot]

Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.

[emcelroy]

Looks good 👍 I left a couple inline comments/questions, but they're non-blocking. The review below generated with Claude raises some issues you may want to address.

---

The design holds up. Splitting assignedLegendAttributeID from legendAttributeIsInoperable is the right call, and the comment at getLegendColorForCase explaining why the check must come first will save the next reader real time. Findings below, most serious first.

---

1. A polygon layer with an unusable legend loses its color control

Make a map from a dataset with one boundary attribute and one ordinary attribute. Move the boundary attribute to a parent collection, so the ordinary attribute is childmost. Assign the ordinary attribute as the legend. The polygon layer is the only candidate, so it now takes the assignment:

layers: [ 'mapPolygonLayer' ]
assigned: pop  inoperable: true  attrType: undefined  isPolygon: true

Now follow the palette. legend-color-controls.tsx:43 sets showShape to false for a polygon, so displayShapeRow is null. Then legend-color-controls.tsx:199 returns that null. The Layers palette shows no color control for the layer.

But the polygon is still drawn in displayItemDescription.itemColor — the deliberate decision recorded at map-polygon-layer.tsx:51. So the color that is still doing the work becomes unreachable. Before this PR, attrType was undefined for that layer and the code fell through to the last branch, which does render the color swatch.

Suggestion. Keep the color control whenever the layer still uses its own color, or gate the early return on "this display grays its items" rather than on legendAttributeIsInoperable alone.

Related test gap. Every new test in legend-color-controls.test.tsx sets pointShapes: "on". That flag is off by default, and with it off showShape is false on a graph too, so the same early return empties the palette section there. That case is defensible — the point color really is inert on a graph — but it is the shipping configuration and nothing covers it.

---

2. Connecting lines still disagree between the map and the graph

map-point-layer.tsx:136:

const getLegendColor = legendAttribute ? dataConfiguration?.getLegendColorForCase : undefined

This feeds renderConnectingLines. On a map with an unusable legend it is undefined, because attributeID('legend') reads "". So use-connecting-lines.ts:124 falls back to pointColorAtIndex — the parent colors. The lines come out colored while the points beside them are gray.

The graph does the opposite. At scatter-plot.tsx:247, legendAttrID is still truthy, so a real getter is passed, getLegendColorForCase returns the missing-value color, and the lines go gray with the points.

This is the same "map contradicts graph" problem the stylePoint fix addresses, and it needs the same treatment.

---

3. The message says "point" on a layer that has no points

en-US.json5:1677 reads "Each point can span multiple %@ values…". The state in finding 1 puts that sentence under a boundary-only map. Either add a second string for boundaries, or word it so it covers both.

---

4. A now-unreachable branch in getLegendColorForCase

data-configuration-model.ts:988 still tests legendCollectionIsMoreChildmost. I do not think anything can reach it now.

To get there you need a truthy attributeID('legend'), a live attribute, and legendAttributeIsInoperable false. The last one means isAttributeAllowedForNonAxisRole returned true, which means the legend's collection index is at or above the childmost index. legendCollectionIsMoreChildmost asks the opposite question against the same childmost index, from the same axisAttributeIDs. The two are exact opposites, so the new check at the top of the function has already returned.

The sibling check at data-configuration-model.ts:1019 in getLegendShapeForCase is still live, because that function returns early on !legendID first. That asymmetry is what will confuse someone later. Either drop the dead branch, or leave a note saying why one stays and one goes.

---

5. One call site missed in the assignedLegendAttributeID sweep

map-content-model.ts:533 still compares the dragged attribute against attributeID('legend'). When the legend is unusable that reads "", so the map reports it will accept a drop of the attribute it already holds. Harmless, but it is the same trap the rest of the PR fixes.

[kswenson]

All five taken, plus both inline suggestions. Pushed as 1747755c0.

1. Polygon color control. Fixed, and your trace was exact — showShape false → displayShapeRow null → the early return hands that null back. It's a regression, as you say: before this PR attrType was undefined and the code fell through to the branch that renders the swatch.

The fix follows from the polygon decision itself. The palette was asking "is the legend honorable" when the question it actually needs is "do the items I describe take the missing-value color" — true for points, false for boundaries, which keep itemColor deliberately. That's now itemsTakeMissingColor, and it's what all three palette gates read. Kirk confirmed the polygon case in the browser.

Your test-gap point is the one I'd have missed on my own. Every palette test I wrote sets pointShapes: "on", and the flag ships off — so the configuration that actually ships took the empty-return path with nothing covering it. Added a test for it. Worth noting that the flag-off graph case is still an empty fill-color control, which is correct (the points really are gray) but now deliberate rather than incidental.

2. Connecting lines. Real, and the same class as the stylePoint fix — a second path testing the filtered attribute ID. The map's getLegendColor now accounts for the inoperable state, so lines go gray with the points they join.

3. Wording. Now "Each item can span multiple %@ values…", since a boundary layer reaches this state.

4. Dead branch. Agreed and removed — isAttributeAllowedForNonAxisRole and legendCollectionIsMoreChildmost are exact opposites over the same childmost index, so the check above it has already returned.

The asymmetry you pointed at was the more useful half. getLegendShapeForCase genuinely still needs its copy, because it has no inoperable check and returns early on an empty legendID. That's now said at the surviving check, so the next reader doesn't have to re-derive why one went and one stayed.

5. Drop gate. Fixed — compares assignedLegendAttributeID.

On coverage, so it's not discovered later: findings 2, 3, 5 and the heatmap tweak have no tests. They're React render paths and a drop predicate with no harness in these directories, and building one is more than this PR should carry. Finding 1, the flag-off case and the model changes are covered and mutation-checked. The uncovered ones plus the four from earlier in the PR were all verified by hand in the browser.