Remove old js components: Card, Table #6560

antgamdia · 2023-08-03T12:08:29Z

Description of the change

This PR is following up #6554 (and requires it to get merged first). In this final one, the focus is on:

Transform the Card and its sub-components to tsx.
Transform the Table to tsx
Remove the remaining test snapshots

Benefits

No cool before/after pics this time :P Here we have just migrated js-based code to typescript.
However, after this PR we will able to say we finally got rid of the legacy js-components!!

Possible drawbacks

N/A - hopefully.

Applicable issues

Related to #6563

Additional information

While transforming the Card to tsx I also had a look at the CdsCard from Clarity. Unfortunately, there is no drop-in replacement; a migration would require plenty of style-related changes. Besides, the "native" style differs a bit from the general style we have in Kubeapps. Below are some examples:

Example from the Clarity website

Quick/dirty replacement, it does not look good

Our current custom card

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Signed-off-by: Antonio Gamez Diaz <[email protected]> Conflicts: dashboard/src/components/AppList/AppListItem.tsx dashboard/src/components/AppView/AccessURLTable/AccessURLTable.tsx dashboard/src/components/AppView/AppControls/RollbackButton/RollbackDialog.test.tsx dashboard/src/components/Config/PkgRepoList/PkgRepoList.tsx dashboard/src/components/SearchFilter/SearchFilter.tsx dashboard/src/components/hooks/useOutsideClick/useOutsideClick.test.tsx dashboard/src/components/js/hooks/useOutsideClick/useOutsideClick.test.js

Signed-off-by: Antonio Gamez Diaz <[email protected]>

netlify · 2023-08-03T12:08:41Z

✅ Deploy Preview for kubeapps-dev canceled.

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`6e92f1b`
🔍 Latest deploy log	https://app.netlify.com/sites/kubeapps-dev/deploys/64d368965504180008ec36b0

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Signed-off-by: Antonio Gamez Diaz <[email protected]> Conflicts: dashboard/src/components/AppView/AppView.tsx dashboard/src/components/Catalog/Catalog.tsx dashboard/src/components/DeploymentForm/DeploymentForm.tsx dashboard/src/components/OperatorInstance/OperatorInstance.tsx dashboard/src/components/OperatorList/OperatorList.tsx dashboard/src/components/OperatorNew/OperatorNew.tsx dashboard/src/components/OperatorView/OperatorView.tsx dashboard/src/components/PackageHeader/PackageView.test.tsx dashboard/src/containers/ConfigLoaderContainer/ConfigLoaderContainer.ts dashboard/src/containers/RoutesContainer/Routes.test.tsx dashboard/src/containers/RoutesContainer/Routes.tsx

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Signed-off-by: Antonio Gamez Diaz <[email protected]> Conflicts: dashboard/src/components/AppView/AppView.tsx dashboard/src/components/Catalog/Catalog.tsx dashboard/src/components/ConfigLoader/ConfigLoader.tsx dashboard/src/components/DeploymentForm/DeploymentForm.tsx dashboard/src/components/Header/ContextSelector.test.tsx dashboard/src/components/OperatorInstance/OperatorInstance.tsx dashboard/src/components/OperatorInstanceForm/OperatorInstanceForm.tsx dashboard/src/components/OperatorInstanceUpdateForm/OperatorInstanceUpdateForm.test.tsx dashboard/src/components/OperatorInstanceUpdateForm/OperatorInstanceUpdateForm.tsx dashboard/src/components/OperatorList/OperatorList.tsx dashboard/src/components/OperatorNew/OperatorNew.test.tsx dashboard/src/components/OperatorNew/OperatorNew.tsx dashboard/src/components/OperatorView/OperatorView.test.tsx dashboard/src/components/OperatorView/OperatorView.tsx dashboard/src/components/PackageHeader/PackageView.test.tsx dashboard/src/components/PackageHeader/PackageView.tsx dashboard/src/components/UpgradeForm/UpgradeForm.tsx dashboard/src/components/js/Button/Button.test.js dashboard/src/containers/ConfigLoaderContainer/ConfigLoaderContainer.ts

Signed-off-by: Antonio Gamez Diaz <[email protected]> Conflicts: dashboard/src/components/AppList/AppListGrid.tsx dashboard/src/components/AppUpgrade/AppUpgrade.test.tsx dashboard/src/components/AppUpgrade/AppUpgrade.tsx dashboard/src/components/AppView/AppControls/RollbackButton/RollbackDialog.tsx dashboard/src/components/AppView/AppView.tsx dashboard/src/components/Catalog/Catalog.tsx dashboard/src/components/ConfirmDialog/ConfirmDialog.tsx dashboard/src/components/DeploymentForm/DeploymentForm.tsx dashboard/src/components/Header/ContextSelector.tsx dashboard/src/components/OperatorInstance/OperatorInstance.tsx dashboard/src/components/OperatorInstanceForm/OperatorInstanceForm.tsx dashboard/src/components/OperatorInstanceUpdateForm/OperatorInstanceUpdateForm.tsx dashboard/src/components/OperatorList/OperatorList.tsx dashboard/src/components/OperatorNew/OperatorNew.tsx dashboard/src/components/OperatorView/OperatorView.tsx dashboard/src/components/PackageHeader/PackageReadme.tsx dashboard/src/components/PackageHeader/PackageView.tsx dashboard/src/components/SelectRepoForm/SelectRepoForm.tsx dashboard/src/components/UpgradeForm/UpgradeForm.tsx dashboard/src/components/js/Button/Button.test.js

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Signed-off-by: Antonio Gamez Diaz <[email protected]> Conflicts: dashboard/src/components/Config/PkgRepoList/PkgRepoList.tsx dashboard/src/components/FilterGroup/__snapshots__/FilterGroup.test.tsx.snap

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Signed-off-by: Antonio Gamez Diaz <[email protected]> Conflicts: dashboard/src/components/Config/PkgRepoList/PkgRepoList.test.tsx dashboard/src/components/Config/PkgRepoList/PkgRepoList.tsx dashboard/src/components/OperatorInstanceUpdateForm/OperatorInstanceUpdateForm.tsx

absoludity

Sorry, I'd missed that this one was still waiting for a review.

absoludity · 2023-08-09T23:31:59Z

dashboard/src/components/InfoCard/Card/CardImage/CardImage.tsx

+// Copyright 2020-2023 the Kubeapps contributors.
+// SPDX-License-Identifier: Apache-2.0
+
+export interface ICardImageProps {
+  src: string;
+  alt: string;
+}
+
+const CardImage = ({ src, alt }: ICardImageProps) => {
+  return (
+    <div className="card-img">
+      <img src={src} alt={alt} />
+    </div>
+  );
+};
+
+export default CardImage;


Almost doesn't seem worth a separate component?

absoludity · 2023-08-09T23:33:13Z

dashboard/src/components/InfoCard/Card/CardText/CardText.tsx

+// Copyright 2020-2023 the Kubeapps contributors.
+// SPDX-License-Identifier: Apache-2.0
+
+import React from "react";
+
+export interface ICardTextProps {
+  children: React.ReactNode;
+}
+
+const CardText = ({ children }: ICardTextProps) => {
+  return <div className="card-text">{children}</div>;
+};
+
+export default CardText;


Same here. Is it a react thing to break the elements up like this? Seems like a /InfoCard/Card could just have the text and image without splitting those out into separate components?

100% agree, no actual benefits, especially if we ever plan to use the card from clarity at some point in the future. I thought the same while moving the code to tsx. I'll make a follow-up PR with the changes shortly.

### Description of the change As pointed out #6560 (comment), we were using plenty of sub-components with no clear motivation. This is to simplify the `InfoCard` component in a single one, as it is just `<div>` and CSS classes. ### Benefits No unused or with extra complexity code. ### Possible drawbacks N/A ### Applicable issues - related #6563 ### Additional information Before/after pics, they look exactly the same, which is what is intended here. ![image](https://github.com/vmware-tanzu/kubeapps/assets/11535726/03bc0a9d-256b-4a42-81fd-98159a181b63) Extra: I'm also squeezing two minor CSS changes: 1) I [ran a tool for detecting unused code](https://www.npmjs.com/package/webpack-deadcode-plugin), and noticed there were some CSS files being inconsistently imported; 2) the secrets view was badly positioned, see below: <details><summary>Secrets view</summary> ![image](https://github.com/vmware-tanzu/kubeapps/assets/11535726/c87e8689-ec1f-4454-91bb-fcfe1a2cdb04) </details> --------- Signed-off-by: Antonio Gamez Diaz <[email protected]>

antgamdia added 30 commits July 28, 2023 14:03

Remove unused checkbox component

9ab9966

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Make SearchFilter use the cds input instead

f2e5582

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Remove unused input component

e669dad

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Replace all tooltips with react-tooltip instead

b1dd6a3

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Remove unused tooltip component

6cfb181

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Merge branch 'main' into remove-js-components

dc95185

Update imports

ea7224e

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Update test cases

2bd380f

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Fix pending test

d37456f

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Update copyright year

b01f040

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Move Row and Column to to tsx components

3f5a5e3

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Update imports

b52ad98

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Update imports

2c402da

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Update imports

1ef4ecc

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Move useOutsideClick to tsx

26eb6b3

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Merge branch 'main' into remove-js-components-1

125e61a

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Remove unnecesary isSomeResourceLoading helper

5fb8be4

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Replace spinner with CdsProgressCircle

ff4ec9e

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Remove unused spinner js component

f9ca909

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Avoid using js button in js alert

c0b4a21

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Remove unused button js component

7b9ec80

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Use AlertGroup instead of the js Alert

79923b4

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Improve AlertGroup

1bfc57e

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Remove unused alert js component

4a24fe8

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Add cancel button in new ns modal

8c457e0

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Replace a remaining plain button with cds one

5ecf38f

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Merge branch 'main' into remove-js-components-1

ef22503

Merge branch 'remove-js-components-1' into remove-js-components-2

1168a2a

Fix alert selector

dddcafc

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Update copyright years

dd173c6

Signed-off-by: Antonio Gamez Diaz <[email protected]>

vmwclabot added the cla-not-required label Aug 3, 2023

antgamdia added 7 commits August 3, 2023 16:38

Fix selector

25df7fd

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Fix selector

887fdb8

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Remove commented test

0fbc571

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Move Table to a tsx component

ad9f1cf

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Rename Table imports

7458395

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Remove remaining snapshots

31bfccc

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Update copyright year

0fb5925

Signed-off-by: Antonio Gamez Diaz <[email protected]>

antgamdia changed the title ~~Remove js components 3~~ Remove old js components: Card, Table Aug 3, 2023

antgamdia added 10 commits August 4, 2023 11:18

Update copyright year

51f86bc

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Run prettier

bf1279e

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Merge branch 'remove-js-components-2'

066717d

Signed-off-by: Antonio Gamez Diaz <[email protected]> Conflicts: dashboard/src/components/Config/PkgRepoList/PkgRepoList.tsx dashboard/src/components/FilterGroup/__snapshots__/FilterGroup.test.tsx.snap

Fix some tests

4dc17ab

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Merge branch 'remove-js-components-2' into remove-js-components-3

f53d619

Fix msg

bf82b69

Signed-off-by: Antonio Gamez Diaz <[email protected]>

antgamdia marked this pull request as ready for review August 7, 2023 08:41

antgamdia added 2 commits August 7, 2023 16:47

Merge branch 'main' into remove-js-components-3

f3f04ff

Merge branch 'main' into remove-js-components-3

6e92f1b

absoludity approved these changes Aug 9, 2023

View reviewed changes

antgamdia mentioned this pull request Aug 10, 2023

Remove old plain JS components #6563

Closed

antgamdia merged commit 9f12c5a into vmware-tanzu:main Aug 10, 2023
38 checks passed

antgamdia deleted the remove-js-components-3 branch August 10, 2023 07:22

antgamdia mentioned this pull request Aug 10, 2023

Remove card subcomponents #6604

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove old js components: Card, Table #6560

Remove old js components: Card, Table #6560

antgamdia commented Aug 3, 2023 •

edited

Loading

netlify bot commented Aug 3, 2023 •

edited

Loading

absoludity left a comment

absoludity Aug 9, 2023

absoludity Aug 9, 2023

antgamdia Aug 10, 2023

Remove old js components: Card, Table #6560

Remove old js components: Card, Table #6560

Conversation

antgamdia commented Aug 3, 2023 • edited Loading

Description of the change

Benefits

Possible drawbacks

Applicable issues

Additional information

netlify bot commented Aug 3, 2023 • edited Loading

✅ Deploy Preview for kubeapps-dev canceled.

absoludity left a comment

Choose a reason for hiding this comment

absoludity Aug 9, 2023

Choose a reason for hiding this comment

absoludity Aug 9, 2023

Choose a reason for hiding this comment

antgamdia Aug 10, 2023

Choose a reason for hiding this comment

antgamdia commented Aug 3, 2023 •

edited

Loading

netlify bot commented Aug 3, 2023 •

edited

Loading