fix(scorecards): correct check output units and related properties - #60
Open
colmcahalane-toast wants to merge 2 commits into
Open
Conversation
Two defects in how check results render, both found by comparing the plugin against DX's own scorecard UI for the same entity. Custom output units were pluralized before display, but the unit is author-supplied and already written in its intended form. `pluralize()` appends "s" to anything outside its special-case list, so units came out doubled: "4 trace metricss", "89 charss". Render the unit verbatim, as DX does. `pluralize()` still serves the duration_* output types, whose units are hardcoded singulars. The related-property section never rendered for any check. The API returns `related_properties` as an array; the type and the drawer both expected a singular `related_property` string, which no response contains. Correct the field and render every property in the array. Also fixes `CustomOutputOptions.unit`, typed as the string literal "string" rather than `string`.
The chip set a light background but left the text color inherited, so dark themes rendered near-white text on it at a 1.1:1 contrast ratio. Pair the background with an explicit foreground, as the check result badges already do.
colmcahalane-toast
marked this pull request as ready for review
August 6, 2026 10:10
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
1 participant
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.
Two rendering defects in scorecard check results, found by comparing the plugin's output against the DX web UI for the same entity, plus a follow-on contrast fix.
Custom output units were pluralised twice
formatCustomOutputValuepassed the author-suppliedunitthroughpluralize(), which appendssto anything outside its special-case list. Units are already written in their intended form, so they came out doubled:trace metrics4 trace metricss4 trace metricschars89 charss89 charsmonitors71 monitorss71 monitors% time > 90% util0.00 % time > 90% utils0.00 % time > 90% utilscreenshot taken with different data,
count difference expected
The unit is now rendered verbatim, matching how DX's own UI displays it.
pluralize()still serves theduration_*output types, whose units are hardcoded singulars (second,minute, …), so1 day/3 daysbehaviour is unchanged.The related-property section never rendered
entities.scorecardsreturnsrelated_propertiesas an array of property identifiers. The type and the drawer both expected a singularrelated_propertystring, which no response contains — so the "Related property / Edit in DX" block was dead code and never appeared for any check, on any entity.Verified against a live API response: of 18 checks on one scorecard, 9 carried a
related_propertiesvalue and none had arelated_propertykey. The field is now correct and every entry in the array is rendered, with the label switching between singular and plural on count.Related-property chips were illegible on dark themes
Once the section above started rendering, the chip turned out to set a light
background-colorwhile leavingcolorinherited — near-white text on#F3F4F6, a contrast ratio of 1.1:1 under a dark theme. It now pairs the background with an explicit foreground, asCheckResultBadgealready does for its status colours, giving 9.4:1.Also included
CustomOutputOptions.unitwas typed as the string literal"string"rather thanstring, making any real unit value a type error.CheckResultBadgeandCheckResultDrawercovering the above. These are the first component tests in the package; each one was confirmed to fail against the unfixed code.Testing
yarn tsc,yarn lint,yarn format:check,yarn testandyarn buildall pass. Also verified in a real Backstage instance (Backstage 1.51, dark theme) by installing the packed tarball, where the badge text and the related-property row both render as expected.These changes were written with Claude Code and reviewed by me before submitting.