Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add header prop to table #2337

Merged
merged 1 commit into from
Feb 19, 2024
Merged

Add header prop to table #2337

merged 1 commit into from
Feb 19, 2024

Conversation

arbulu89
Copy link
Contributor

@arbulu89 arbulu89 commented Feb 19, 2024 •
edited
Loading

Description

Add header prop to Table. This allows us to include a header to the table using the correct bordering layout.
It will be used in the HanaClusterDetails view, to border properly the site details.

Besides that, the table borders are rounded now on.

How was this tested?

The styles tested with Storybook

@arbulu89 arbulu89 added enhancement New feature or request javascript Pull requests that update Javascript code labels Feb 19, 2024
@arbulu89 arbulu89 requested review from dottorblaster, nelsonkopliku, rtorrero, CDimonaco and EMaksy and removed request for nelsonkopliku February 19, 2024 13:35
rtorrero
rtorrero approved these changes Feb 19, 2024
View reviewed changes
Copy link
Contributor

@rtorrero rtorrero left a comment

Choose a reason for hiding this comment

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

Thanks @arbulu89

For some context: this came up during the review for #2328 where we noticed the rounded corners where being applied on the header always, even when the table is contained in another diff, and this looks odd. Check the PR for a screenshot.

EMaksy
EMaksy approved these changes Feb 19, 2024
View reviewed changes
Copy link
Member

@EMaksy EMaksy left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@rtorrero rtorrero mentioned this pull request Feb 19, 2024
Merged
dottorblaster
dottorblaster approved these changes Feb 19, 2024
View reviewed changes
Copy link
Contributor

@dottorblaster dottorblaster left a comment

Choose a reason for hiding this comment

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

LGTM, beware of that onliner! But really up to you if it's something to be refactored 😄

assets/js/common/Table/Table.jsx Show resolved Hide resolved
@arbulu89 arbulu89 merged commit 2c9056e into main Feb 19, 2024
32 checks passed
@arbulu89 arbulu89 deleted the add-header-table branch February 19, 2024 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dottorblaster dottorblaster dottorblaster approved these changes

@EMaksy EMaksy EMaksy approved these changes

@rtorrero rtorrero rtorrero approved these changes

@CDimonaco CDimonaco Awaiting requested review from CDimonaco

Assignees
No one assigned
Labels
enhancement New feature or request javascript Pull requests that update Javascript code
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@arbulu89 @dottorblaster @rtorrero @EMaksy