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.

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

feat: Introduce resizable table component #2606

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ironAiken2
Copy link
Contributor

@ironAiken2 ironAiken2 commented Aug 6, 2024

TL;DR

Introduce BAITable, resizable table component.

What changed?

  • Implemented a table component that can be resized. Usage is the same as Table in traditional antd.

BAITable calculates and applies new widths based on a fixed width, so there are two rules to follow for smooth behavior

  • BAITable must have a width specified in the column data.
  • If you use data that exceeds the width specified for the column, you must use the ellipsis property.

How to test?

  • Install the newly added packages.
  • Replace the parts of the webui that currently use table with BAITable.

Checklist: (if applicable)

  • Mention to the original issue
  • Documentation
  • Minium required manager version
  • Specific setting for review (eg., KB link, endpoint or how to setup)
  • Minimum requirements to check during review
  • Test case(s) to demonstrate the difference of before/after

Copy link

graphite-app bot commented Aug 6, 2024

Your org requires the Graphite merge queue for merging into main

Add the label “flow:merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “flow:hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@github-actions github-actions bot added the size:L 100~500 LoC label Aug 6, 2024
Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @ironAiken2 and the rest of your teammates on Graphite Graphite

@ironAiken2 ironAiken2 force-pushed the feat/resizable-table branch 5 times, most recently from c48f351 to 57f6cab Compare August 9, 2024 02:38
Copy link

github-actions bot commented Aug 9, 2024

Coverage report for ./react

St.
Category Percentage Covered / Total
🔴 Statements
5.39% (-0.02% 🔻)
338/6272
🔴 Branches
4.91% (-0.01% 🔻)
214/4357
🔴 Functions
3.05% (-0.01% 🔻)
63/2063
🔴 Lines
5.29% (-0.02% 🔻)
324/6125
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🔴
... / BAITable.tsx
0% 0% 0% 0%

Test suite run success

90 tests passing in 11 suites.

Report generated by 🧪jest coverage report action from 7dfa157

@ironAiken2 ironAiken2 marked this pull request as ready for review August 9, 2024 02:46
Copy link
Contributor

@agatha197 agatha197 left a comment

Choose a reason for hiding this comment

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

Sorry for checking late. Please solve the conflicts

@github-actions github-actions bot added size:XL 500~ LoC and removed size:L 100~500 LoC labels Aug 12, 2024
@github-actions github-actions bot added size:L 100~500 LoC and removed size:XL 500~ LoC labels Aug 12, 2024
Copy link
Contributor

@agatha197 agatha197 left a comment

Choose a reason for hiding this comment

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

If I set the width to overflow, that column looks like this.
image.png
But, when I added this code to the columns, you can avoid overflow. Can you generalize it?

style={{
            overflow: 'hidden',
            whiteSpace: 'pre',
            wordWrap: 'break-word',
          }}

@ironAiken2 ironAiken2 force-pushed the feat/resizable-table branch 2 times, most recently from 22cd94b to b1587fb Compare August 26, 2024 05:24
Copy link
Contributor

@agatha197 agatha197 left a comment

Choose a reason for hiding this comment

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

image

If I set the narrow width, I cannot see the part of the contents until widening that column.
By adding whiteSpace: 'pre', wordWrap: 'break-word' to https://github.com/lablup/backend.ai-webui/pull/2606/files#diff-20fe13a09e30cb99672ad684d2c592b13f7ba6c973a8f0ec7a208195e4e3edefR22 line, you can avoid it or you can add a new line or add ellipsis, etc.

react/src/components/KeypairResourcePolicyList.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@agatha197 agatha197 left a comment

Choose a reason for hiding this comment

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

@ironAiken2 ironAiken2 marked this pull request as draft August 30, 2024 09:47
Copy link
Contributor Author

Currently, the way vaadin tables show contents on resize is a mix of the line wrapping method you mentioned and the overflow: hidden method currently implemented.

The overflow:hidden method seems like a good idea because it doesn't break the table layout, but we'll have to talk about how to proceed. @agatha197 @yomybaby

image.png

@ironAiken2 ironAiken2 marked this pull request as ready for review September 2, 2024 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:L 100~500 LoC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants