[UI] Add Virtualise List for Volume Table #2938

ChengYanJin · 2020-11-20T10:17:41Z

Component: ui, tables

Context:
With the use case of thousands of volumes, the table would meet performance issues when rendering them at once.
So instead of doing this, we just render a small subset of rows at a give time.

Summary:
Introduce react-window and react-virtualized-auto-sizer to realize the virtualize row.
Adapt the interaction test

UI change:
Replace the "Unknown" text in the table with a hyphen icon to reduce the noise that less important information brings.

Acceptance criteria:

If we open the developer tool => Elements, we should see just a few of the <tr/> at the beginning, when we scroll down on the list, we can see the update of the list.

Closes: #2931

bert-e · 2020-11-20T10:17:43Z

Hello chengyanjin,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Status report is not available.

bert-e · 2020-11-20T10:17:46Z

Branches have diverged

This pull request's source branch improvement/add-virtualise-list-for-tables has diverged from
development/2.6 by more than 50 commits.

To avoid any integration risks, please re-synchronize them using one of the
following solutions:

Merge origin/development/2.6 into improvement/add-virtualise-list-for-tables
Rebase improvement/add-virtualise-list-for-tables onto origin/development/2.6

Note: If you choose to rebase, you may have to ask me to rebuild
integration branches using the reset command.

bert-e · 2020-11-20T10:24:00Z

Integration data created

I have created the integration data for the additional destination branches.

this pull request will merge improvement/add-virtualise-list-for-tables into
development/2.6
w/2.7/improvement/add-virtualise-list-for-tables will be merged into development/2.7

The following branches will NOT be impacted:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5

You can set option create_pull_requests if you need me to create
integration pull requests in addition to integration branches, with:

@bert-e create_pull_requests

bert-e · 2020-11-20T10:24:02Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

bert-e · 2020-11-20T10:25:50Z

History mismatch

Merge commit #d8a6d63191aa0fd356244edda4009768c8f5f684 on the integration branch
w/2.7/improvement/add-virtualise-list-for-tables is merging a branch which is neither the current
branch improvement/add-virtualise-list-for-tables nor the development branch
development/2.7.

It is likely due to a rebase of the branch improvement/add-virtualise-list-for-tables and the
merge is not possible until all related w/* branches are deleted or updated.

Please use the reset command to have me reinitialize these branches.

thomasdanan

works well, just you have to mouse over very precisely on the hyphen to see the tooltip, may be the zone could be enlarged a bit

ChengYanJin · 2020-11-25T11:36:42Z

works well, just you have to mouse over very precisely on the hyphen to see the tooltip, may be the zone could be enlarged a bit

Done! Thanks for the feedback!

ChengYanJin · 2020-12-09T14:09:20Z

/approve

bert-e · 2020-12-09T14:09:25Z

History mismatch

Merge commit #d8a6d63191aa0fd356244edda4009768c8f5f684 on the integration branch
w/2.7/improvement/add-virtualise-list-for-tables is merging a branch which is neither the current
branch improvement/add-virtualise-list-for-tables nor the development branch
development/2.7.

It is likely due to a rebase of the branch improvement/add-virtualise-list-for-tables and the
merge is not possible until all related w/* branches are deleted or updated.

Please use the reset command to have me reinitialize these branches.

The following options are set: approve

ChengYanJin · 2020-12-09T14:09:36Z

/reset

bert-e · 2020-12-09T14:09:45Z

Reset complete

I have successfully deleted this pull request's integration branches.

The following options are set: approve

bert-e · 2020-12-09T14:09:55Z

Integration data created

I have created the integration data for the additional destination branches.

this pull request will merge improvement/add-virtualise-list-for-tables into
development/2.6
w/2.7/improvement/add-virtualise-list-for-tables will be merged into development/2.7

The following branches will NOT be impacted:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5

You can set option create_pull_requests if you need me to create
integration pull requests in addition to integration branches, with:

@bert-e create_pull_requests

The following options are set: approve

bert-e · 2020-12-09T14:59:30Z

Build failed

The build for commit did not succeed in branch w/2.7/improvement/add-virtualise-list-for-tables.

The following options are set: approve

bert-e · 2020-12-23T10:46:02Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

The following options are set: approve

Autosizer automatically adjusts the width and height of a single child. Refs: #2931 ui/table: Add flex for table row to fully use the space Refs: #2931

Too many "Unknown" in the table seems extremly noisy and since the "Unknown" information is rather less important, we should replace it by an icon with tooltip. Refs: #2931

- Upgrade the react-table version to get support for react (v17.0.1) - Introduce react-window and react-virtualized-auto-sizer for the virtualize list Refs: #2931

For non-specified instance URL, such as `/volumes`, we should automatically choose the first one in the list. However, since we are not aware currectVolumeName in VolumePage component, we fall into update the URL which triger the VolumePage re-renders. Cause the infinite API calls. So I move this logic to VolumePageContent which solve the issue. Refs: #2931

Refs: #2931

Since <AutoSizer> is a <div/> so it breaks the table layout, we need to use <div/> for all the HTML tags in table (thead, tbody, tr, td...) and retrieve the defaullt styles by className. Refs: #2931

So that the users don't need to hover on the hypen precisely Refs: #2931

bert-e · 2021-01-06T10:00:31Z

Integration data created

I have created the integration data for the additional destination branches.

this pull request will merge improvement/add-virtualise-list-for-tables into
development/2.7
w/2.8/improvement/add-virtualise-list-for-tables will be merged into development/2.8

The following branches will NOT be impacted:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6

You can set option create_pull_requests if you need me to create
integration pull requests in addition to integration branches, with:

@bert-e create_pull_requests

The following options are set: approve

bert-e · 2021-01-06T10:00:32Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

The following options are set: approve

Remove the unused styled for HTML elements

Refs: #2931

JBWatenbergScality · 2021-01-06T11:07:47Z

ui/src/components/TableRow.js

+`;
+
+const TableRow = (props) => {
+  const { row, style, rowClicked, volumeName, theme } = props;


I would rename rowClicked props to onClick to match standard event names

Instead of passing volumeName I would introduce an isSelected prop that reflect in a clearer way if the row is selected or not

In order to avoid props drilling of the theme you can rely on styled component which automatically map the theme as part of the props or use useTheme hook that retrieve current theme from styled-component internal context

JBWatenbergScality · 2021-01-06T11:23:10Z

ui/src/components/VolumeListTable.js

@@ -151,6 +100,13 @@ const TooltipContent = styled.div`
  min-width: 60px;
 `;

+const UnknownIcon = styled.i`


This component is duplicated in table row, please consider exporting it instead if it is necessary

JBWatenbergScality · 2021-01-06T11:24:38Z

ui/src/components/VolumeListTable.js

+    ({ index, style }) => {
+      const row = rows[index];
+      prepareRow(row);
+      // Extract the TableRow component outside of Table component


This comment can be deleted

JBWatenbergScality · 2021-01-06T11:25:52Z

ui/src/components/VolumeListTable.js

-                {intl.translate('no_volume_found')}
-              </td>
-            </HeadRow>
+                No Volume


A translation seems to be missing here, maybe {intl.translate('no_volume_found')}

To match standard event names Refs: #2931

Refs: #2931

Since style component can map the theme as part of the props. we don't need to pass the theme props to the component. Refs: #2931

bert-e · 2021-01-06T15:09:45Z

Build failed

The build for commit did not succeed in branch improvement/add-virtualise-list-for-tables.

The following options are set: approve

bert-e · 2021-01-07T13:21:45Z

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

✔️ development/2.7
✔️ development/2.8

The following branches will NOT be impacted:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

Any commit you add on the source branch will trigger a new cycle after the
current queue is merged.
Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

bert-e · 2021-01-08T08:33:39Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/2.7
✔️ development/2.8

The following branches have NOT changed:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6

Please check the status of the associated issue None.

Goodbye chengyanjin.

ChengYanJin requested review from MonPote, thomasdanan and gdemonet November 20, 2020 10:17

ChengYanJin requested a review from a team as a code owner November 20, 2020 10:17

ChengYanJin force-pushed the improvement/add-virtualise-list-for-tables branch 2 times, most recently from 09fd05f to 66f38c0 Compare November 20, 2020 10:23

ChengYanJin force-pushed the improvement/add-virtualise-list-for-tables branch from 66f38c0 to 2ee989f Compare November 20, 2020 10:25

ChengYanJin force-pushed the improvement/add-virtualise-list-for-tables branch 5 times, most recently from 328355a to 9163f97 Compare November 24, 2020 19:50

thomasdanan previously approved these changes Nov 25, 2020

View reviewed changes

thomasdanan requested review from alexis-ld and nicolas2bert November 25, 2020 07:16

thomasdanan reviewed Nov 25, 2020

View reviewed changes

ChengYanJin dismissed thomasdanan’s stale review via 8f0ca15 November 25, 2020 11:35

thomasdanan previously approved these changes Dec 9, 2020

View reviewed changes

ChengYanJin force-pushed the improvement/add-virtualise-list-for-tables branch 2 times, most recently from ee34d84 to 734b1a7 Compare December 23, 2020 10:45

ChengYanJin changed the base branch from development/2.6 to development/2.7 December 23, 2020 10:45

ChengYanJin force-pushed the improvement/add-virtualise-list-for-tables branch from 734b1a7 to c1fa73b Compare December 23, 2020 17:46

ChengYanJin added 7 commits January 6, 2021 10:55

ui/table: Use the react-window to virtualize the lines

659bce2

Autosizer automatically adjusts the width and height of a single child. Refs: #2931 ui/table: Add flex for table row to fully use the space Refs: #2931

ui/table: Replace the text unknown by hyphen icon

2b7c339

Too many "Unknown" in the table seems extremly noisy and since the "Unknown" information is rather less important, we should replace it by an icon with tooltip. Refs: #2931

ui/package: Update the UI packages

958ecc6

- Upgrade the react-table version to get support for react (v17.0.1) - Introduce react-window and react-virtualized-auto-sizer for the virtualize list Refs: #2931

ui/cypress: Fix the failed test caused by the table virtualization

d514475

Refs: #2931

ui/tables: Replace all the html tags in table with div

1b9a8b7

Since <AutoSizer> is a <div/> so it breaks the table layout, we need to use <div/> for all the HTML tags in table (thead, tbody, tr, td...) and retrieve the defaullt styles by className. Refs: #2931

ui/volumes: Increase the height of Unknown icon

08eabfc

So that the users don't need to hover on the hypen precisely Refs: #2931

ChengYanJin force-pushed the improvement/add-virtualise-list-for-tables branch from c1fa73b to 8f871bc Compare January 6, 2021 10:00

ChengYanJin added 2 commits January 6, 2021 11:00

ui/volumeTable: Wrap the word for each of the cell

527ca60

Remove the unused styled for HTML elements

ui/volumes: Extract the TableRow component

8f871bc

Refs: #2931

ChengYanJin requested a review from JBWatenbergScality January 6, 2021 10:01

ChengYanJin added 2 commits January 6, 2021 11:42

ui/table: Extract the TableRow outside of VolumeListTable

5a6e28b

Refs: #2931

ui/table: Pass the isSelected to the TableRow instead of volumeName

9d8e0ff

Refs: #2931

JBWatenbergScality reviewed Jan 6, 2021

View reviewed changes

ChengYanJin added 3 commits January 6, 2021 12:40

ui/table: Rename the rowClicked to onClick for table

4ded403

To match standard event names Refs: #2931

ui/table: Retrieve the common TableRow styled from its component

9cbd1e2

Refs: #2931

ui/table: Remove the theme props which pass to the components

9e46321

Since style component can map the theme as part of the props. we don't need to pass the theme props to the component. Refs: #2931

JBWatenbergScality approved these changes Jan 6, 2021

View reviewed changes

bert-e merged commit b87b113 into development/2.7 Jan 8, 2021

bert-e deleted the improvement/add-virtualise-list-for-tables branch January 8, 2021 08:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[UI] Add Virtualise List for Volume Table #2938

[UI] Add Virtualise List for Volume Table #2938

ChengYanJin commented Nov 20, 2020 •

edited

Loading

bert-e commented Nov 20, 2020

bert-e commented Nov 20, 2020

bert-e commented Nov 20, 2020

bert-e commented Nov 20, 2020

bert-e commented Nov 20, 2020

thomasdanan left a comment

ChengYanJin commented Nov 25, 2020

ChengYanJin commented Dec 9, 2020

bert-e commented Dec 9, 2020

ChengYanJin commented Dec 9, 2020

bert-e commented Dec 9, 2020

bert-e commented Dec 9, 2020

bert-e commented Dec 9, 2020

bert-e commented Dec 23, 2020

bert-e commented Jan 6, 2021

bert-e commented Jan 6, 2021

JBWatenbergScality Jan 6, 2021

JBWatenbergScality Jan 6, 2021

JBWatenbergScality Jan 6, 2021

JBWatenbergScality Jan 6, 2021

JBWatenbergScality Jan 6, 2021

JBWatenbergScality Jan 6, 2021

bert-e commented Jan 6, 2021

bert-e commented Jan 7, 2021

bert-e commented Jan 8, 2021

[UI] Add Virtualise List for Volume Table #2938

[UI] Add Virtualise List for Volume Table #2938

Conversation

ChengYanJin commented Nov 20, 2020 • edited Loading

bert-e commented Nov 20, 2020

Hello chengyanjin,

bert-e commented Nov 20, 2020

Branches have diverged

bert-e commented Nov 20, 2020

Integration data created

bert-e commented Nov 20, 2020

Waiting for approval

bert-e commented Nov 20, 2020

History mismatch

thomasdanan left a comment

Choose a reason for hiding this comment

ChengYanJin commented Nov 25, 2020

ChengYanJin commented Dec 9, 2020

bert-e commented Dec 9, 2020

History mismatch

ChengYanJin commented Dec 9, 2020

bert-e commented Dec 9, 2020

Reset complete

bert-e commented Dec 9, 2020

Integration data created

bert-e commented Dec 9, 2020

Build failed

bert-e commented Dec 23, 2020

Waiting for approval

bert-e commented Jan 6, 2021

Integration data created

bert-e commented Jan 6, 2021

Waiting for approval

JBWatenbergScality Jan 6, 2021

Choose a reason for hiding this comment

JBWatenbergScality Jan 6, 2021

Choose a reason for hiding this comment

JBWatenbergScality Jan 6, 2021

Choose a reason for hiding this comment

JBWatenbergScality Jan 6, 2021

Choose a reason for hiding this comment

JBWatenbergScality Jan 6, 2021

Choose a reason for hiding this comment

JBWatenbergScality Jan 6, 2021

Choose a reason for hiding this comment

bert-e commented Jan 6, 2021

Build failed

bert-e commented Jan 7, 2021

In the queue

bert-e commented Jan 8, 2021

ChengYanJin commented Nov 20, 2020 •

edited

Loading