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.

Sign up for GitHub

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

Jump to bottom

UI: Refactor to use functional components #9810

Open
52 of 53 tasks
tczhao opened this issue Oct 13, 2022 · 21 comments · Fixed by #10867 or #10925
Open
52 of 53 tasks

UI: Refactor to use functional components #9810

tczhao opened this issue Oct 13, 2022 · 21 comments · Fixed by #10867 or #10925
Assignees
@agilgur5
Labels
area/ui solution/suggested A solution to the bug has been suggested. Someone needs to implement it. type/feature Feature request type/tech-debt

Comments

@tczhao
Copy link
Member

tczhao commented Oct 13, 2022 •
edited by agilgur5
Loading

Summary

Say for UI features, new feature added in workflow page would likely be useful in archived workflow as well,
but right now we have both class and functional components at various parts of the UI, porting features from one part to another can be a pain.

class component file list:

--- below heavily modified by agilgur5 ---

plan to refactor them in batches

Message from the maintainers:

Love this enhancement proposal? Give it a 👍. We prioritise the proposals with the most 👍.
@tczhao tczhao added the type/feature Feature request label Oct 13, 2022
@tczhao tczhao mentioned this issue Oct 13, 2022
Merged
8 tasks
@terrytangyuan
Copy link
Member

@tczhao Would you like to pick this up?

@terrytangyuan
Copy link
Member

There are also other good UI/UX related issues if you want to help: https://github.com/orgs/argoproj/projects/29

@tczhao
Copy link
Member Author

tczhao commented Apr 7, 2023

@tczhao Would you like to pick this up?

Sure :)

@tczhao
Copy link
Member Author

tczhao commented Apr 7, 2023 •
edited
Loading

There are also other good UI/UX related issues if you want to help: https://github.com/orgs/argoproj/projects/29

Looks good, thanks for the link

@tczhao tczhao self-assigned this Apr 7, 2023
@tczhao tczhao mentioned this issue Apr 10, 2023
Merged
alexec pushed a commit that referenced this issue Apr 11, 2023
@tczhao
chore: refactor app.tsx. Fixes #9810 (#10867)
28a1822 
Signed-off-by: Tianchu Zhao <evantczhao@gmail.com>
@alexec alexec reopened this Apr 11, 2023
@tczhao tczhao mentioned this issue Apr 16, 2023
Merged
@terrytangyuan terrytangyuan reopened this Apr 17, 2023
JPZ13 pushed a commit to pipekit/argo-workflows that referenced this issue Jul 4, 2023
@tczhao @JPZ13
chore: refactor app.tsx. Fixes argoproj#9810 (argoproj#10867)
c59db06 
Signed-off-by: Tianchu Zhao <evantczhao@gmail.com>
@agilgur5 agilgur5 mentioned this issue Sep 4, 2023
Merged
@agilgur5
Copy link
Member

agilgur5 commented Sep 4, 2023 •
edited
Loading

  • 8. ui/src/app/shared/components/checkbox-filter/checkbox-filter.tsx:

Completed CheckboxFilter as part of #11739

@agilgur5 agilgur5 mentioned this issue Sep 9, 2023
Merged
@agilgur5
Copy link
Member

agilgur5 commented Sep 9, 2023 •
edited
Loading

  • 10. ui/src/app/shared/components/error-panel.tsx:
  • 14. ui/src/app/shared/components/inline-table/inline-table.tsx:
  • 16. ui/src/app/shared/components/nudge.tsx:
  • 17. ui/src/app/shared/components/pagination-panel.tsx:
  • 20. ui/src/app/shared/conditions-panel.tsx:
  • 22. ui/src/app/shared/resources-duration.tsx:
  • 37. ui/src/app/workflows/components/workflow-creator-info/workflow-creator-info.tsx:
  • 38. ui/src/app/workflows/components/workflow-dag/workflow-dag-render-options-panel.tsx:
  • 42. ui/src/app/workflows/components/workflow-labels/workflow-labels.tsx:
  • 43. ui/src/app/workflows/components/workflow-panel/workflow-panel.tsx:
  • 45. ui/src/app/workflows/components/workflow-yaml-viewer/workflow-yaml-viewer.tsx:
  • 47. ui/src/app/workflows/components/workflows-row/workflows-row.tsx:

Completed the above 12 in #11791

Also completed one that was missing from the list:

  • 49. ui/src/app/workflows/components/workflow-node-info/workflow-node-info.tsx

@agilgur5
Copy link
Member

agilgur5 commented Sep 9, 2023 •
edited
Loading

Some of the files listed in the opening comment are also not React components. Quick way to tell: they end in .ts and not .tsx.

I think they may have been pulled into the list by just searching class, but those are regular classes, not React components. Searching React.Component would be more accurate.

Checking all 16 off:

  • 11. ui/src/app/shared/components/graph/coffman-graham-sorter.ts:
  • 12. ui/src/app/shared/components/graph/dfs-sorter.ts:
  • 13. ui/src/app/shared/components/graph/types.ts:
  • 21. ui/src/app/shared/list-watch.ts:
  • 23. ui/src/app/shared/retry-observable.ts:
  • 24. ui/src/app/shared/retry-watch.ts:
  • 25. ui/src/app/shared/scoped-local-storage.ts:
  • 26. ui/src/app/shared/services/archived-workflows-service.ts:
  • 27. ui/src/app/shared/services/cluster-workflow-template-service.ts:
  • 28. ui/src/app/shared/services/cron-workflow-service.ts:
  • 29. ui/src/app/shared/services/event-service.ts:
  • 30. ui/src/app/shared/services/event-source-service.ts:
  • 31. ui/src/app/shared/services/info-service.ts:
  • 32. ui/src/app/shared/services/sensor-service.ts:
  • 33. ui/src/app/shared/services/workflow-template-service.ts:
  • 34. ui/src/app/shared/services/workflows-service.ts:

@agilgur5
Copy link
Member

agilgur5 commented Sep 9, 2023 •
edited
Loading

  • 36. ui/src/app/workflows/components/submit-workflow-panel.tsx:
  • 50. ui/src/app/workflows/components/resubmit-workflow-panel.tsx:
  • 51. ui/src/app/workflows/components/retry-workflow-panel.tsx:

@toyamagu-2021 do you think you'd be able to refactor the submit/resubmit/retry workflow panels to functional components? Since you had created the latter two, I figure you're most familiar with them 🙂

EDIT: these have been completed in #11803

Feel free to take any others on as well if you'd like!

@toyamagu-2021
Copy link
Member

toyamagu-2021 commented Sep 10, 2023 •
edited
Loading

OK, I'll do that!
（Please wait a few days since my colleage might be interested in them :）)

This was referenced Sep 10, 2023
Merged
Merged
@agilgur5
Copy link
Member

agilgur5 commented Sep 10, 2023 •
edited
Loading

  • 2. ui/src/app/archived-workflows/components/archived-workflow-details/archived-workflow-details.tsx:
  • 3. ui/src/app/archived-workflows/components/archived-workflow-filters/archived-workflow-filters.tsx:
  • 4. ui/src/app/archived-workflows/components/archived-workflow-list/archived-workflow-list.tsx:

These 3 components no longer exist as the Archived Workflows UI was merged with the Workflows UI in #11121
(and neither does the archived-workflow-service, but that was already checked off above as it is not a React component)

  • 52. ui/src/app/shared/components/error-boundary.tsx

@agilgur5
Copy link
Member

agilgur5 commented Sep 10, 2023 •
edited
Loading

  • 6. ui/src/app/reports/components/reports.tsx:
  • 7. ui/src/app/shared/components/base-page.tsx:
  • 35. ui/src/app/userinfo/components/user-info.tsx:
  • 41. ui/src/app/workflows/components/workflow-filters/workflow-filters.tsx:
  • 46. ui/src/app/workflows/components/workflows-list/workflows-list.tsx:

Completed Reports and UserInfo in #11793 and #11794. Partially removed BasePage in those as well.

Both of these used to use BasePage, as does WorkflowsList. Once WorkflowsList is migrated, BasePage can be deleted entirely (which also unblocks react-router-dom upgrades).

I'm working on a PR for WorkflowsList as well, which is one of the most complicated components in the codebase (if not the most). There are some other PRs currently open for WorkflowsList as well though, so would want those to be merged soon so that we have less merge conflicts to resolve.

@agilgur5 agilgur5 mentioned this issue Sep 11, 2023
Merged
@agilgur5
Copy link
Member

  • 18. ui/src/app/shared/components/resource-editor/resource-editor.tsx:
  • 19. ui/src/app/shared/components/tags-input/tags-input.tsx:
  • 40. ui/src/app/workflows/components/workflow-drawer/workflow-drawer.tsx:

Completed these 3 components in #11800

This was referenced Sep 12, 2023
Closed
Merged
@agilgur5 agilgur5 mentioned this issue Sep 22, 2023
Closed
8 tasks
@agilgur5 agilgur5 self-assigned this Sep 23, 2023
@agilgur5
Copy link
Member

agilgur5 commented Sep 23, 2023 •
edited
Loading

  • 9. ui/src/app/shared/components/dropdown/dropdown.tsx:
  • 15. ui/src/app/shared/components/input-filter.tsx:
  • 39. ui/src/app/workflows/components/workflow-dag/workflow-dag.tsx:
  • 44. ui/src/app/workflows/components/workflow-timeline/workflow-timeline.tsx:
  • 48. ui/src/app/workflows/components/workflows-toolbar/workflows-toolbar.tsx:

Would anyone like to take some or all of these remaining 5? (Please write which you want to take if so)
EDIT: Completed in #11899, #11901, #11920, #12046

We've also got another new one that was missed in #11803 (I didn't realize it existed so didn't have it in the initial list, my bad!):

cc @tetora1053 @juijeong8324 @yunwoo-yu who may be interested 🙂

@juijeong8324
Copy link
Contributor

  • 9. ui/src/app/shared/components/dropdown/dropdown.tsx:
  • 15. ui/src/app/shared/components/input-filter.tsx:
  • 39. ui/src/app/workflows/components/workflow-dag/workflow-dag.tsx:
  • 44. ui/src/app/workflows/components/workflow-timeline/workflow-timeline.tsx:
  • 48. ui/src/app/workflows/components/workflows-toolbar/workflows-toolbar.tsx:

Would anyone like to take some or all of these remaining 5? (Please write which you want to take if so)

We've also got another new one that was missed in #11803 (I didn't realize it existed so didn't have it in the initial list, my bad!):

  • 53. ui/src/app/shared/components/parameters-input/parameters-input.tsx

cc @tetora1053 @juijeong8324 @yunwoo-yu who may be interested 🙂

Oh I'll do that!! I'm interested in 9, 48 and 53!

  • 9. ui/src/app/shared/components/dropdown/dropdown.tsx:
  • 48. ui/src/app/workflows/components/workflows-toolbar/workflows-toolbar.tsx:
  • 53. ui/src/app/shared/components/parameters-input/parameters-input.tsx

I will take 53 first! If anyone interested in 9 or 48, it's okay take them😀😀

@yunwoo-yu
Copy link
Contributor

  • 9. ui/src/app/shared/components/dropdown/dropdown.tsx:
  • 15. ui/src/app/shared/components/input-filter.tsx:
  • 39. ui/src/app/workflows/components/workflow-dag/workflow-dag.tsx:
  • 44. ui/src/app/workflows/components/workflow-timeline/workflow-timeline.tsx:
  • 48. ui/src/app/workflows/components/workflows-toolbar/workflows-toolbar.tsx:

Would anyone like to take some or all of these remaining 5? (Please write which you want to take if so)

We've also got another new one that was missed in #11803 (I didn't realize it existed so didn't have it in the initial list, my bad!):

  • 53. ui/src/app/shared/components/parameters-input/parameters-input.tsx

cc @tetora1053 @juijeong8324 @yunwoo-yu who may be interested 🙂

Hello 😄

  • 9. ui/src/app/shared/components/dropdown/dropdown.tsx:

I'm interested in all of them! But I'll start with number 9 in the list order 👍.

@agilgur5 agilgur5 mentioned this issue Sep 27, 2023
Merged
@agilgur5
Copy link
Member

  • 7. ui/src/app/shared/components/base-page.tsx:
  • 41. ui/src/app/workflows/components/workflow-filters/workflow-filters.tsx:
  • 46. ui/src/app/workflows/components/workflows-list/workflows-list.tsx:

Completed these all in #11891. That was actually a lot more challenging than I expected and is probably one of the most involved refactors I've done in several years 😮
It legit made my head hurt at times fixing bugs and getting the semantics to be equivalent 😅 (to be fair, I do have chronic migraines 🤕 )

That was the most difficult refactor in the UI codebase though -- which is why I took it on myself -- so with that over, it's just the final stretch of the remaining 5. Hopefully new contributors above can tackle those effectively 😉

@tetora1053
Copy link
Contributor

  • 9. ui/src/app/shared/components/dropdown/dropdown.tsx:
  • 15. ui/src/app/shared/components/input-filter.tsx:
  • 39. ui/src/app/workflows/components/workflow-dag/workflow-dag.tsx:
  • 44. ui/src/app/workflows/components/workflow-timeline/workflow-timeline.tsx:
  • 48. ui/src/app/workflows/components/workflows-toolbar/workflows-toolbar.tsx:

Would anyone like to take some or all of these remaining 5? (Please write which you want to take if so)

We've also got another new one that was missed in #11803 (I didn't realize it existed so didn't have it in the initial list, my bad!):

  • 53. ui/src/app/shared/components/parameters-input/parameters-input.tsx

cc @tetora1053 @juijeong8324 @yunwoo-yu who may be interested 🙂

ok! I take 15, 39, 44 (no one has taken those yet, right?)

@juijeong8324 juijeong8324 mentioned this issue Sep 27, 2023
Merged
@juijeong8324
Copy link
Contributor

juijeong8324 commented Sep 28, 2023 •
edited
Loading

@tetora1053 yeah you are right!
I'm almost done 53, so I will take 48!

@tetora1053 tetora1053 mentioned this issue Sep 28, 2023
Merged
@tetora1053
Copy link
Contributor

I'll tackle no.39 with another PR. (cause I'm afraid it might take a little longer than the other two)

@yunwoo-yu yunwoo-yu mentioned this issue Sep 29, 2023
Merged
@tetora1053 tetora1053 mentioned this issue Sep 30, 2023
Open
@juijeong8324
Copy link
Contributor

juijeong8324 commented Oct 7, 2023 •
edited
Loading

Hello, I'm sorry to be late in tackling number 48 (I had a cold and wasn't feeling well...) I'll open the PR within this week!! Thank you!!

@agilgur5
Copy link
Member

agilgur5 commented Oct 13, 2023 •
edited
Loading

no worries I was sick this week with a pretty bad cold too 😷 (and am behind on some reviews as such) . hope you are feeling better!

This was referenced Oct 15, 2023
Closed
Merged
@agilgur5 agilgur5 added the solution/suggested A solution to the bug has been suggested. Someone needs to implement it. label Jan 7, 2024
@agilgur5 agilgur5 mentioned this issue Feb 1, 2024
Closed
7 tasks
@agilgur5 agilgur5 changed the title Refactor UI to use functional component UI: Refactor to use functional components Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@agilgur5 agilgur5

Labels
area/ui solution/suggested A solution to the bug has been suggested. Someone needs to implement it. type/feature Feature request type/tech-debt
Projects
Argo Workflows UI/UX Improvements
Status: In Progress
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore: refactor app.tsx. Fixes #9810 tczhao/argo
chore: refactor ui share service. tczhao/argo
8 participants
@alexec @terrytangyuan @agilgur5 @tczhao @tetora1053 @juijeong8324 @toyamagu-2021 @yunwoo-yu