RS-22593: Expose pictograph value as ChartData and title as ChartLabels$ChartTitle - #15
Closed
JustinCCYap wants to merge 2 commits into
Closed
RS-22593: Expose pictograph value as ChartData and title as ChartLabels$ChartTitle#15JustinCCYap wants to merge 2 commits into
JustinCCYap wants to merge 2 commits into
Conversation
…ls$ChartTitle VisualizeNumber and PictographChart rendered their value (and, for VisualizeNumber, a title via text.above) but did not expose them on the output, so consumers reading attr(x, "ChartData") / attr(x, "ChartLabels") (e.g. the LLM-readable generation) had nothing to read or label. VisualizeNumber now attaches the input value as ChartData and text.above as ChartLabels$ChartTitle (when non-empty), applied to both of its return paths. PictographChart attaches its cleaned data matrix as ChartData; it has no title. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the wrapper helper with direct attribute assignment at each return path. The title to expose is computed once (NULL when no title is shown), so each path sets ChartData and ChartLabels on its widget without nesting calls. Co-Authored-By: Claude Opus 4.8 (1M context) <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.
Summary
VisualizeNumber, a title viatext.above) but did not expose them on the output object, so consumers readingattr(x, "ChartData")/attr(x, "ChartLabels")$ChartTitle— notably the LLM-readable generation used by Chat to answer questions from existing outputs — had nothing to read or label.VisualizeNumbernow sets the input value asChartDataandtext.aboveasChartLabels$ChartTitle(computed once,NULLwhen no title is shown) directly on the widget at each of its return paths (main and icon/pictograph).PictographChartattaches its cleaned data matrix asChartData(captured before the internal rescale for icon rendering). It has no title, so noChartLabels.Jira: RS-22593
Test plan
test-visualizenumber.R: assertsChartDataandChartLabels$ChartTitleare exposed on both return paths, and that no title is exposed whentext.aboveis empty.test-pictographchart.R: assertsChartDatamatches the input values and noChartLabelsis set.AI code etiquette
VisualizeNumberchange touches control flow (two return paths), so it warrants closer review than the pictograph/PSM data attachment.🤖 Generated with Claude Code