✨ integrate column management feature into apps table #1767

ibolton336 · 2024-03-13T02:08:54Z

Resolves #1765

ibolton336 · 2024-03-14T14:25:17Z

#1765

rszwajko

Some comments below (based on experience gathered from kubev2v/forklift-console-plugin#64 and follow-ups)

client/src/app/hooks/table-controls/column/useColumnState.ts

client/src/app/hooks/table-controls/useTableControlProps.ts

client/src/app/pages/applications/applications-table/applications-table.tsx

ibolton336 · 2024-03-21T14:45:04Z

Screen.Recording.2024-03-21.at.10.43.45.AM.mov

sjd78

Overall the change LGTM. A few comments, and only 1 console.log that should be taken out.

client/src/app/hooks/table-controls/useLocalTableControls.ts

sjd78 · 2024-03-25T16:11:38Z

client/src/app/pages/applications/applications-table/applications-table.tsx

-                <Th {...getThProps({ columnKey: "tags" })} width={10} />
-                <Th {...getThProps({ columnKey: "effort" })} width={10} />
+                {/* {columnState.columns. isVisible then render this column} */}
+                {getColumnVisibility("name") && (


I don't mind this approach.

The visibility prop exists and is used in the PF docs to use the css breakpoints to show/hide columns. Fun fact is that prop is not in any of the generated docs and it needs to be tracked down in sources. I tried that and got lost pretty quick. Probably isn't worth trying to use visibility for the user based show/hide column support.

sjd78 · 2024-03-25T16:15:21Z

client/src/app/hooks/table-controls/column/useColumnState.ts

+  const [columns, setColumns] = useState<ColumnState<TColumnKey>[]>(
+    args.initialColumns
+  );
+  return { columns, setColumns };


And this is where you'd have to add column visibility persistence past the component state level that is used now. I predict there will be a QE ticket filed in less than 10 minutes of app testing say that the column visibility doesn't persist page page refreshes.

A chat before testing would help. Maybe even loop docs in to level set expectations as to how long the column management state lives.

Added local storage support for the column names.

sjd78

Now that I'm running it to play around, I have 2 comments, one of which needs to be resolved for sure:

The column name in the list is not center aligned (needs to be fixed)
I highly suggest that the Name column is setup to be required. It can show up on the list, and the checkbox checked, but the checkbox being disabled.

Screen shot from FF:

client/src/app/pages/applications/applications-table/components/manage-columns-toolbar.tsx

client/src/app/pages/applications/applications-table/components/manage-columns-modal.tsx

client/src/app/pages/applications/applications-table/components/manage-columns-toolbar.tsx

Signed-off-by: Ian Bolton <[email protected]>

ibolton336 · 2024-03-25T21:10:21Z

Updated to match pr suggestions / UXD suggestions.

sjd78

Looks really good. The only highly recommended change would be for the i18next strings in the ManageColumnsToolbar.

client/src/app/pages/applications/applications-table/components/manage-columns-modal.tsx

client/src/app/pages/applications/applications-table/components/manage-columns-toolbar.tsx

Signed-off-by: Ian Bolton <[email protected]>

rszwajko

Looks OK to me. Any open points can be addressed in follow-ups.

sjd78

LGTM

ibolton336 requested review from sjd78 and rszwajko March 14, 2024 17:44

sjd78 linked an issue Mar 14, 2024 that may be closed by this pull request

Integrate col management to applications table #1765

Closed

rszwajko reviewed Mar 18, 2024

View reviewed changes

mturley mentioned this pull request Mar 18, 2024

Port over new features/fixes added to tackle2-ui's table-controls since this repo diverged from it patternfly-labs/react-table-batteries#11

Open

ibolton336 force-pushed the manage-columns-modal branch 2 times, most recently from 316bb70 to 076f9a2 Compare March 21, 2024 14:43

ibolton336 marked this pull request as ready for review March 21, 2024 14:45

ibolton336 force-pushed the manage-columns-modal branch from 076f9a2 to d10dbac Compare March 21, 2024 14:45

ibolton336 changed the title ~~WIP start integrating column name management tool into apps table~~ ✨ integrate column management feature into apps table Mar 21, 2024

ibolton336 closed this Mar 22, 2024

ibolton336 reopened this Mar 22, 2024

ibolton336 force-pushed the manage-columns-modal branch 3 times, most recently from 5ee0e04 to 139b48b Compare March 22, 2024 19:14

sjd78 approved these changes Mar 25, 2024

View reviewed changes

sjd78 requested changes Mar 25, 2024

View reviewed changes

rszwajko reviewed Mar 25, 2024

View reviewed changes

✨ Integrate col mgmt modal feature with table hooks

e2a0503

Signed-off-by: Ian Bolton <[email protected]>

ibolton336 force-pushed the manage-columns-modal branch from fee201f to e2a0503 Compare March 25, 2024 19:55

sjd78 reviewed Mar 26, 2024

View reviewed changes

client/src/app/pages/applications/applications-table/components/manage-columns-modal.tsx Show resolved Hide resolved

client/src/app/pages/applications/applications-table/components/manage-columns-toolbar.tsx Outdated Show resolved Hide resolved

ibolton336 added 2 commits March 26, 2024 13:57

PR suggestions

797d152

Signed-off-by: Ian Bolton <[email protected]>

PR suggestions

7d245ed

Signed-off-by: Ian Bolton <[email protected]>

ibolton336 force-pushed the manage-columns-modal branch from 463ee9a to 7d245ed Compare March 26, 2024 17:57

ibolton336 requested review from rszwajko and sjd78 March 26, 2024 17:58

Merge branch 'main' into manage-columns-modal

57156c5

rszwajko approved these changes Mar 27, 2024

View reviewed changes

sjd78 approved these changes Mar 27, 2024

View reviewed changes

ibolton336 merged commit c2361a9 into konveyor:main Mar 27, 2024
6 checks passed

This was referenced May 8, 2024

operator konveyor-operator (0.4.0-alpha.1) k8s-operatorhub/community-operators#4324

Merged

operator konveyor-operator (0.4.0-alpha.1) redhat-openshift-ecosystem/community-operators-prod#4509

Merged

This was referenced Jun 5, 2024

operator konveyor-operator (0.4.0) k8s-operatorhub/community-operators#4455

Merged

operator konveyor-operator (0.4.0) redhat-openshift-ecosystem/community-operators-prod#4623

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ integrate column management feature into apps table #1767

✨ integrate column management feature into apps table #1767

ibolton336 commented Mar 13, 2024 •

edited

Loading

ibolton336 commented Mar 14, 2024

rszwajko left a comment

ibolton336 commented Mar 21, 2024

sjd78 left a comment

sjd78 Mar 25, 2024

sjd78 Mar 25, 2024

ibolton336 Mar 25, 2024

sjd78 left a comment

ibolton336 commented Mar 25, 2024

sjd78 left a comment

rszwajko left a comment

sjd78 left a comment

✨ integrate column management feature into apps table #1767

✨ integrate column management feature into apps table #1767

Conversation

ibolton336 commented Mar 13, 2024 • edited Loading

ibolton336 commented Mar 14, 2024

rszwajko left a comment

Choose a reason for hiding this comment

ibolton336 commented Mar 21, 2024

sjd78 left a comment

Choose a reason for hiding this comment

sjd78 Mar 25, 2024

Choose a reason for hiding this comment

sjd78 Mar 25, 2024

Choose a reason for hiding this comment

ibolton336 Mar 25, 2024

Choose a reason for hiding this comment

sjd78 left a comment

Choose a reason for hiding this comment

ibolton336 commented Mar 25, 2024

sjd78 left a comment

Choose a reason for hiding this comment

rszwajko left a comment

Choose a reason for hiding this comment

sjd78 left a comment

Choose a reason for hiding this comment

ibolton336 commented Mar 13, 2024 •

edited

Loading