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

Remove old js components: Checkbox, Input, Tooltip #6539

Merged
merged 10 commits into from Aug 1, 2023

Conversation

antgamdia
Copy link
Contributor

@antgamdia antgamdia commented Jul 28, 2023

Description of the change

This is to replace some old js components we used to have. They were created before the ones provided by Clarity existed.
For making this PR somehow bearable, I'm just adding the following changes:

  • Checkbox: it wasn't used at all, just removing the files:
  • Input: just used by the SearchFilter, so I've replaced it with the preferred <CdsInput>.
  • Tooltip: we moved to react-tooltip, but there were some occurrences of this custom component. I have replaced each one and fixed the unit tests.

Benefits

Have a look at the following before/after pics:

Input

input-before

input-after

Tooltip 1

tooltip-before

tooltip-after

Tooltip 2

tooltip1-before

tooltip1-after

Tooltip 3

tooltip2-before

tooltip2-after

Tooltip 4

tooltip3-before

tooltip3-after

Tooltip 5

tooltip4-before

tooltip4-after

Possible drawbacks

Applicable issues

Can't recall the long-term issue we had for migrating to CDS :S

Related to #6563

Additional information

The remaining js components are:

  1. Alert (52 occurrences)
  2. Card (1 occurrence)
  3. CardBlock (2 occurrences)
  4. CardFooter (1 occurrence)
  5. CardHeader (1 occurrence)
  6. Column (23 occurrences)
  7. Row (26 occurrences)
  8. Table (5 occurrences)
  9. TableRow (1 occurrences)
  10. useOutsideClick (2 occurrences)

They will be either replaced (ex Alert->AlertGroup, maybe) or migrated to typescript if possible.

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
@netlify
Copy link

netlify bot commented Jul 28, 2023

Deploy Preview for kubeapps-dev canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit b01f040
🔍 Latest deploy log https://app.netlify.com/sites/kubeapps-dev/deploys/64c7dda54d39680008d1ecf4

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
@antgamdia antgamdia marked this pull request as ready for review July 31, 2023 16:13
@antgamdia antgamdia changed the title Remove old js components Remove old js components: Checkbox, Input, Tooltip Jul 31, 2023
Copy link
Contributor

@absoludity absoludity left a comment

Choose a reason for hiding this comment

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

Did I say already how much I miss your PRs :) (so helpful thinking about exactly what people want to know - in this case, before and after shots).

Thanks for adding the missing acts in the test too!

@antgamdia antgamdia merged commit e758ada into vmware-tanzu:main Aug 1, 2023
39 checks passed
@antgamdia antgamdia deleted the remove-js-components branch August 1, 2023 11:17
antgamdia added a commit that referenced this pull request Aug 4, 2023
### Description of the change

This PR is following up #6539. In this one, the focus is on:

- Transforming `Row` and `Column` to tsx components.
- Update some imports
- Transform the `useOutsideClick` hook to typescript.
- I saw there is a [library with useful
hooks](https://usehooks.com/useclickaway), but since we just have this
one, adding yet another dep maybe is not worth it.
- Remove an unnecessary `isSomeResourceLoading` helper and its folder
structure

After those changes, I decided to stop and continue in another PR (the
upcoming changes are more extensive than these ones).

### Benefits

I don't have any cool before/after shots in this PR :P, but I do have a
minor improvement in the `useOutsideClick`: now it also responds to
touch events:


https://github.com/vmware-tanzu/kubeapps/assets/11535726/b13d2e68-9fbc-4ab1-87ad-9593a17d2370


### Possible drawbacks

N/A (hope any!)

### Applicable issues

Related to #6563

### Additional information

N/A

---------

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants