Repository navigation
feat: Enabled support for more than 4 columns for key-value pair - #5081
vikaskale2722 wants to merge 6 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5081 +/- ##
==========================================
- Coverage 97.75% 97.75% -0.01%
==========================================
Files 1003 1003
Lines 32539 32536 -3
Branches 12085 12084 -1
==========================================
- Hits 31810 31807 -3
Misses 684 684
Partials 45 45 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Note
Copilot was unable to run its full agentic suite in this review.
Copilot review overview
3 open issues
The public docs now state that up to 12 columns are supported and higher values are treated as 12,… · New Given the updated docs: “higher values are treated as 12”, add a test case that renders with… · New The comment statesminColumnWidthis always passed to forceFlexibleColumnLayout, but the… · New
What changed in this PR
Updates KeyValuePairs to support more columns (beyond the previous hard limit), adjusts documentation to reflect the new columns behavior, and updates tests and permutations accordingly.
Changes:
- Removed the runtime warning and clamping logic that limited
columnsto 4. - Updated public props documentation for
columns/minColumnWidthand refreshed generated docs snapshot. - Reworked tests and added new visual permutations for higher column counts.
| File | Description |
|---|---|
| src/key-value-pairs/internal.tsx | Removes the 4-column cap/warn and passes columns directly to ColumnLayout; updates a11y-related comment. |
| src/key-value-pairs/interfaces.ts | Updates API docs to state support up to 12 columns and describes responsive reduction behavior. |
| src/key-value-pairs/__tests__/key-value-pairs.test.tsx | Replaces warn-on-exceed tests with “no warn” assertions and adds layout assertions for several column counts. |
| src/__tests__/snapshot-tests/__snapshots__/documenter.test.ts.snap | Updates generated docs snapshot to match new prop documentation. |
| pages/key-value-pairs/permutations.page.tsx | Adds permutations covering 5–6 columns and different minColumnWidth values. |
Review effort: Lite
| variant="text-grid" | ||
| minColumnWidth={minColumnWidth} | ||
| > | ||
| <ColumnLayout __tagOverride="dl" columns={columns} variant="text-grid" minColumnWidth={minColumnWidth}> |
There was a problem hiding this comment.
What if a consumer passes minColumnWidth={0}
There was a problem hiding this comment.
It will internally use GridColumnLayout (instead of FlexibleColumnLayout which it uses with minColumnWidth) and beyond 4 columns it will cause unexpected behavior. But passing 0 as a input to the column width does not make sense.
Do you think we should add a guardrail?


Description
The Key-Value component only supported 4 columns before this implementation. This PR enables support for more than 4 columns in the component. There was a default hard-coded MAX_COLUMN value defined as 4. We remove the hardcoded maximum default. Earlier, if the number of columns provided were > 4, a warning was published on the console that the maximum columns allowed are 4. This implementation removes the warning as well.
Related links, issue #62223
How has this been tested?
Unit test
Manual test
Dry Run Build : #8046485265
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.