Skip to content
This repository was archived by the owner on May 1, 2025. It is now read-only.

Add Table Story#489

Merged
itamargiv merged 14 commits intoadd-table-tokensfrom
table-story
Sep 28, 2021
Merged

Add Table Story#489
itamargiv merged 14 commits intoadd-table-tokensfrom
table-story

Conversation

@guergana
Copy link
Copy Markdown
Contributor

This PR creates a Story for the Table component added in this commit.

Bug: T291077

@github-actions
Copy link
Copy Markdown

Comment thread vue-components/src/components/Table.vue Outdated
Comment thread vue-components/src/components/Table.vue Outdated
Comment thread vue-components/stories/Table.stories.ts Outdated
Comment thread vue-components/stories/Table.stories.ts Outdated
<td data-header="Vegetable">
{{cell.col1}}
</td>
<td data-header="Column 2">{{cell.col2}}</td>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value of data header should be displayed, this is how the table works...

Comment thread vue-components/src/components/Table.vue Outdated
Comment thread vue-components/src/components/Table.vue Outdated
Copy link
Copy Markdown
Member

@Silvan-WMDE Silvan-WMDE left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me and works great on chromatic

Comment thread vue-components/src/components/Table.vue Outdated
Comment thread vue-components/src/components/Table.vue Outdated
Comment on lines +106 to +108
// Ensure headers stay exactly 40%
// even if values are wider than 60%
min-width: 40%;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Co-authored-by: Itamar Givon <itamar.givon.dev@gmail.com>
In tables, numbers should be right-eligned in order to make them easy to compare. This is something that implenters could achieve in context. In order to avoid having to make any adjustments to the story, we could use a different example that doesn't include numbers, so we don't display a suboptimal design decision.
Comment thread vue-components/src/components/Table.vue Outdated
Comment thread vue-components/src/components/Table.vue Outdated
Comment thread vue-components/src/components/Table.vue Outdated
@itamargiv
Copy link
Copy Markdown
Member

What an all encompassing collaboration! Well done everyone 👏 👏

Copy link
Copy Markdown
Contributor

@sai-san sai-san left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@itamargiv itamargiv merged commit 9c4afd7 into add-table-tokens Sep 28, 2021
@itamargiv itamargiv deleted the table-story branch September 28, 2021 13:40
sai-san added a commit that referenced this pull request Sep 28, 2021
* Add body-component line-height

This removes the label style and adds a token to reduce the line-height of "body" so this can be used as the default style by all components.

* Update ToggleButton.json

* 💥 Breaking changes 💥

* Add table component tokens

* Update Table.json

* Correct line-height

* Align small line-height with design guidelines

* Update Table.json

* Update Table.json

Wondering whether those height tokens should be renamed altogether

* Update tokens/properties/components/Table.json

* Add Table Story (#489)

This PR creates a Story for the Table component added in this commit.

* Add Table Story
* Add min width for normalized column header

Co-authored-by: Itamar Givon <itamar.givon.dev@gmail.com>
Co-authored-by: Silvan Heintze <59574251+Silvan-WMDE@users.noreply.github.com>
Co-authored-by: SaiSan <sarai.sanchez@wikimedia.de>
Bug: T291077

Co-authored-by: Guergana Tzatchkova <guergana.tzatchkova@wikimedia.de>
Co-authored-by: Itamar Givon <itamar.givon.dev@gmail.com>
Co-authored-by: Silvan Heintze <59574251+Silvan-WMDE@users.noreply.github.com>
jakobw pushed a commit that referenced this pull request Nov 4, 2021
* Add body-component line-height

This removes the label style and adds a token to reduce the line-height of "body" so this can be used as the default style by all components.

* Update ToggleButton.json

* 💥 Breaking changes 💥

* Add table component tokens

* Update Table.json

* Correct line-height

* Align small line-height with design guidelines

* Update Table.json

* Update Table.json

Wondering whether those height tokens should be renamed altogether

* Update tokens/properties/components/Table.json

* Add Table Story (#489)

This PR creates a Story for the Table component added in this commit.

* Add Table Story
* Add min width for normalized column header

Co-authored-by: Itamar Givon <itamar.givon.dev@gmail.com>
Co-authored-by: Silvan Heintze <59574251+Silvan-WMDE@users.noreply.github.com>
Co-authored-by: SaiSan <sarai.sanchez@wikimedia.de>
Bug: T291077

Co-authored-by: Guergana Tzatchkova <guergana.tzatchkova@wikimedia.de>
Co-authored-by: Itamar Givon <itamar.givon.dev@gmail.com>
Co-authored-by: Silvan Heintze <59574251+Silvan-WMDE@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants