fix(table-skeleton): colsAlign index start at 1 - #5246
Conversation
|
🚀 Storybook preview deployed: https://pub-dc6d99acd6874e2aaff6219dd8a13ae2.r2.dev/PR-5246/index.html |
|
Can you give us a bit more details on why this is needed please? We use 0-based indexes everywhere, using 1 just here feels wrong. |
|
@Supamiu I already discussed this with Jérémie a couple of months ago, it's so uncomfortable when I know that the fourth column of the table needs to be aligned to the right. I have to think about how it's calculated in the component code, then subtract one to set the correct value that matches the code logic rather than the human reading logic. |
There was a problem hiding this comment.
🟡 Changes recommended
The breaking behavior needs an explicit one-based API description and a component changelog entry.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates table skeletons so colsAlign uses one-based column numbers.
Changes:
- Generates column identifiers from
1throughcols. - Updates templates and examples for one-based alignment keys.
- Adds
colsAligninput documentation.
File summaries
| File | Description |
|---|---|
stories/qa/skeleton-components/skeleton-components.stories.html |
Updates QA examples. |
stories/documentation/loaders/skeleton-table/angular/skeleton-table.stories.ts |
Updates table documentation. |
stories/documentation/loaders/skeleton-table/angular/skeleton-table-body.stories.ts |
Updates body-only example. |
stories/documentation/loaders/skeleton-index-table/angular/skeleton-index-table.stories.ts |
Updates index-table documentation. |
stories/documentation/loaders/skeleton-index-table/angular/skeleton-index-table-body.stories.ts |
Updates body-only example. |
stories/documentation/loaders/skeleton-data-table/angular/skeleton-data-table.stories.ts |
Updates data-table documentation. |
stories/documentation/loaders/skeleton-data-table/angular/skeleton-data-table-body.stories.ts |
Updates body-only example. |
packages/ng/skeleton/skeleton-table/skeleton-table.component.ts |
Generates one-based column numbers. |
packages/ng/skeleton/skeleton-table/skeleton-table.component.html |
Uses column numbers for alignment. |
packages/ng/skeleton/skeleton-index-table/skeleton-index-table.component.ts |
Generates one-based column numbers. |
packages/ng/skeleton/skeleton-index-table/skeleton-index-table.component.html |
Uses column numbers for alignment. |
packages/ng/skeleton/skeleton-data-table/skeleton-data-table.component.ts |
Generates one-based column numbers. |
packages/ng/skeleton/skeleton-data-table/skeleton-data-table.component.html |
Uses column numbers for alignment. |
Review details
- Files reviewed: 13/13 changed files
- Comments generated: 4
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
8de19c7 to
d854cf1
Compare
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Description
🔥 Breaking
The
colsAlignkeys onlu-skeleton-table,lu-skeleton-index-tableandlu-skeleton-data-tableare now column numbers starting at1instead of0-based indexes.Increment every key by one:
{ 0: 'center', 1: 'end' }becomes{ 1: 'center', 2: 'end' }.