Dashboard: Refactor containers to be functional components

Created on 12 Apr 2021  路  10Comments  路  Source: tektoncd/dashboard

Refactor the containers to be functional components so that we can use the React Hooks API.

This is a pre-requisite for adopting react-query and other libraries which only provide a Hooks-based API.

  • [x] About
  • [x] App
  • [x] ClusterTasks
  • [x] ClusterTasksDropdown
  • [x] ClusterTriggerBinding
  • [x] ClusterTriggerBindings
  • [x] Condition
  • [x] Conditions
  • [x] CreatePipelineResource
  • [x] CreatePipelineRun
  • [x] CreateTaskRun
  • [x] CustomResourceDefinition
  • [x] EventListener
  • [x] EventListeners
  • [x] Extension N/A
  • [x] ImportResources
  • [x] PipelineResource
  • [x] PipelineResources
  • [x] PipelineResourcesDropdown
  • [x] PipelineRun
  • [x] PipelineRuns
  • [x] Pipelines
  • [x] PipelinesDropdown
  • [x] ResourceList
  • [x] ServiceAccountsDropdown
  • [x] SideNav
  • [x] TaskRun
  • [x] TaskRuns
  • [x] Tasks
  • [x] TasksDropdown
  • [x] TriggerBinding
  • [x] TriggerBindings
  • [x] TriggerTemplate
  • [x] TriggerTemplates

This was started in https://github.com/tektoncd/dashboard/pull/1984

kincleanup kinmisc

All 10 comments

Do you mind if I want to try refactoring on Tasks container component. :)

@wingyplus that would be great, thank you

@AlanGreene I just see that Tasks container lack of tests and storybook. Do we need to add tests (and storybook) before begin refactoring it?

The storybook is primarily focused on developing and testing the presentational components (from packages/components) in isolation, so there's no need to add an entry for a page-level container.

Many of the containers don't currently have tests as they follow a very similar pattern and usually just interact with the redux code which is already well tested. If there are particular pieces you feel would benefit from additional tests feel free to add those.

Also fyi, we have a custom hook that can be used to trigger updates on websocket reconnect, you can see an example of it being used in https://github.com/tektoncd/dashboard/blob/cdf5be07e426d5bd8cb7bcf14e04a687c73ddaf1/src/containers/ClusterTasks/ClusterTasks.js#L76. This ensures the callback is only executed on reconnect (i.e. where the websocket was previously connected, got disconnected, and then connects again), not on the initial connection which is generally handled by a different useEffect based on the resource name / filters / etc.

Many of the containers don't currently have tests as they follow a very similar pattern and usually just interact with the redux code which is already well tested. If there are particular pieces you feel would benefit from additional tests feel free to add those.

@AlanGreene Thanks for your guide. After apply useWebSocketReconnected, I didn't see any point to add more test to do this ^^.

And I open PR https://github.com/tektoncd/dashboard/pull/2022 for a Tasks component. Please kindly review. ^^

I'm working on TaskRuns.

@wingyplus thank you for your work on this so far. Are you still planning on contributing a PR to refactor the TaskRuns container? If not that's absolutely fine, just let me know so I can complete the remaining work for this issue.

@AlanGreene I've a draft pull request here https://github.com/tektoncd/dashboard/pull/2039. Just converted a few days ago but didn't test it well.

The last PR has finally been merged 馃帀 Now the real work can begin.
/close

@AlanGreene: Closing this issue.

In response to this:

The last PR has finally been merged 馃帀 Now the real work can begin.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Was this page helpful?
0 / 5 - 0 ratings