feat(scorecard): implement blueprint architecture for grouped layouts#3282
feat(scorecard): implement blueprint architecture for grouped layouts#3282Eswaraiahsapram wants to merge 4 commits into
Conversation
Eswaraiahsapram
requested review from
a team,
christoph-jerolimov and
its-mitesh-kumar
as code owners
|
Important This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior. Changed Packages
|
ReviewFindingsLow
Info
Previous runReviewFindingsHigh
Medium
Low
Info
Previous run (2)ReviewReason: stale-head The review agent reviewed commit |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| * extensions (grid + list) into the Catalog entity pages. | ||
| * | ||
| * The plugin ships both layouts; platform engineers enable/disable individual | ||
| * layouts via app-config.yaml. No separate module registration is needed |
There was a problem hiding this comment.
[medium] behavioral-contract-change
scorecardCatalogModule now bundles layout extensions that change the default UI for existing consumers. When layouts are registered, the entity tab renders ScorecardLayoutSwitcher instead of the legacy EntityScorecardContent.
Suggested fix: Ship layout extensions in a separate opt-in module or ensure they are disabled by default.
| success: '#e8f5e9', | ||
| warning: '#fff3e0', | ||
| error: '#fce4ec', | ||
| }; |
There was a problem hiding this comment.
[medium] hardcoded-colors
statusTileBg and statusTileText use hardcoded hex colors that will not adapt to dark mode or custom themes. Rest of codebase uses resolveStatusColor() through the theme.
Suggested fix: Derive colors from MUI theme palette or use alpha(resolvedColor, 0.12) for background tints.
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3282 +/- ##
==========================================
- Coverage 53.62% 53.59% -0.03%
==========================================
Files 2409 2414 +5
Lines 86633 86731 +98
Branches 23997 24031 +34
==========================================
+ Hits 46457 46486 +29
- Misses 39894 39963 +69
Partials 282 282
*This pull request uses carry forward flags. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
|
||
| /** @alpha */ | ||
| export interface ScorecardLayoutItem { | ||
| title: string; |
There was a problem hiding this comment.
[low] edge-case
The fallback layouts[selectedIndex] ?? layouts[0] masks a potential stale-index state. In practice layouts are static extension inputs resolved at boot time, but adding a useEffect to clamp selectedIndex would make the component more robust.
Suggested fix: Add useEffect(() => { if (selectedIndex >= layouts.length) setSelectedIndex(0); }, [layouts.length, selectedIndex]);
| return ( | ||
| <Box> | ||
| {layouts.length > 1 && ( | ||
| <Box display="flex" justifyContent="flex-end" mb={2}> |
There was a problem hiding this comment.
[low] error-handling
Toggle buttons use layout.title as React key. Duplicate titles would cause reconciliation issues. Currently mitigated by hardcoded unique titles.
fullsend-ai-review
Bot
added
the
ready-for-merge
2 participants
Hey, I just made a Pull Request!
Fix - https://redhat.atlassian.net/browse/RHIDP-13955
Description:
Add blueprint pattern for scorecard metric grouping with dynamic layout extensions discoverable through Backstage NFS.
Changes:
Screenshot
How to Test
Config to Enable Both Layouts
✔️ Checklist