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

chore: use Canonical react components #80

Merged
merged 18 commits into from
Sep 23, 2024
Merged

chore: use Canonical react components #80

merged 18 commits into from
Sep 23, 2024

Conversation

gruyaume
Copy link
Collaborator

@gruyaume gruyaume commented Sep 16, 2024

Description

Use Canonical react components instead of having custom html components.

Rationale

This change allows:

  • Using clever behaviors from those components like password validation.
  • Standardisation with the ecosystem of Canonical app
  • Reducing lines of code. Here most of the added code is in the package.json, the net change is closer to -400
  • Less time spent figuring out Canonical layout

I believe now's the right time to do this change before this project grows.

Visual Improvements

This change comes with some visual improvements.

In each image:

  • Left: Current
  • Right: Proposed

Better login and initialization pages

Instead of coding our own layout and logic for the login and initialization pages, we now use canonical's LoginPageLayout which reduces the amount of code by a bunch and looks nice.

image

image

A show CSR content action

We removed the additional action button that shows certificate content and replaced it with an additional action in the action menu named "Show CSR Content". This leverages Canonical MainTable's expanded row feature instead of coding our own.

image

Better input validation visuals

We leverage the "error" parameter in Input components automatically rendering the boxes red and using the appropriate icon instead of coding our own logic. It also looks nicer.

image

image

What's next?

Yes, there are more aspects of the code that could be converted to using react components. Here I did a large first batch of changes. There will be more to come, namely:

  • The App Navigation
  • Asides

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that validate the behaviour of the software
  • I validated that new and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@gruyaume gruyaume changed the title chore: use Canonical react components chore: use Canonical react components (Initialize) Sep 16, 2024
@gruyaume gruyaume marked this pull request as ready for review September 16, 2024 16:05
@gruyaume gruyaume requested a review from a team as a code owner September 16, 2024 16:05
@gruyaume gruyaume marked this pull request as draft September 16, 2024 16:25
@gruyaume gruyaume changed the title chore: use Canonical react components (Initialize) chore: use Canonical react components (Initialize and Login) Sep 16, 2024
@gruyaume gruyaume marked this pull request as ready for review September 16, 2024 16:48
@gruyaume gruyaume changed the title chore: use Canonical react components (Initialize and Login) chore: use Canonical react components Sep 16, 2024
@gruyaume gruyaume changed the title chore: use Canonical react components chore: use Canonical react components (pt.1) Sep 16, 2024
@gruyaume gruyaume marked this pull request as draft September 16, 2024 18:11
@gruyaume gruyaume changed the title chore: use Canonical react components (pt.1) chore: use Canonical react components Sep 16, 2024
@gruyaume gruyaume marked this pull request as ready for review September 16, 2024 21:15
@gruyaume gruyaume marked this pull request as draft September 16, 2024 23:15
@gruyaume gruyaume marked this pull request as ready for review September 17, 2024 11:37
Copy link
Contributor

@kayra1 kayra1 left a comment

Choose a reason for hiding this comment

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

Great progress on this refactor

ui/src/app/nav_account.tsx Outdated Show resolved Hide resolved
ui/src/app/certificate_requests/row.tsx Outdated Show resolved Hide resolved
ui/src/app/certificate_requests/row.tsx Outdated Show resolved Hide resolved
ui/src/app/certificate_requests/components.tsx Outdated Show resolved Hide resolved
ui/src/app/certificate_requests/row.tsx Outdated Show resolved Hide resolved
ui/src/app/certificate_requests/table.tsx Outdated Show resolved Hide resolved
ui/src/app/certificate_requests/table.tsx Outdated Show resolved Hide resolved
ui/src/app/certificate_requests/table.tsx Show resolved Hide resolved
ui/src/app/users/asideForm.tsx Show resolved Hide resolved
ui/src/app/users/table.tsx Show resolved Hide resolved
Signed-off-by: guillaume <[email protected]>
@gruyaume gruyaume merged commit 9d3b09d into main Sep 23, 2024
13 checks passed
@gruyaume gruyaume deleted the dev-react branch September 23, 2024 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants