Skip to content
This repository has been archived by the owner on Jan 16, 2022. It is now read-only.

DetailContainer Component - Replaced class by func. comp #130

Merged
merged 10 commits into from
Oct 3, 2019

Conversation

priscilawebdev
Copy link
Contributor

Type: Refactor

The following has been addressed in the PR: https://github.com/verdaccio/ui/issues/116

Unit or Functional tests are included in the PR?

Not completely

Description:

in order to use react hooks, in this PR I updated the DetailContainer component by changing it from a class to a functional component. I have also splitted it's content so that it is clearer.

I didn't add tests using the react testing library now, because the content of a tab can diverse according to the value passing from the context. I need to think how I will test it, but feedbacks are already very welcome :-)

@juanpicado
Copy link
Member

Test are failing, could you check @priscilawebdev https://app.circleci.com/jobs/github/verdaccio/ui/1522 ?

@juanpicado juanpicado self-requested a review September 29, 2019 11:31
@priscilawebdev
Copy link
Contributor Author

priscilawebdev commented Oct 1, 2019

@juanpicado I have increased the PR's size, (if you prefer I can split the PR) but tests are passing 🙂

Copy link
Member

@sergiohgz sergiohgz left a comment

Choose a reason for hiding this comment

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

Should we differ between components and material Ui components in two different folders? I don't think so

@priscilawebdev
Copy link
Contributor Author

@sergiohgz I would like to start organizing the components in order to implement our Theme...after they will be together again..i just find this way easier for now...

@sergiohgz
Copy link
Member

@priscilawebdev perfect if we use it as workaround for organizing, but I think this approach should not be permanent 😉
Also, this PRs looks so great 😉

@priscilawebdev priscilawebdev changed the title DetailContainer Component - Replaced classes by func. comp DetailContainer Component - Replaced class by func. comp Oct 1, 2019
@juanpicado
Copy link
Member

@sergiohgz I would like to start organizing the components in order to implement our Theme...after they will be together again..i just find this way easier for now...

Let's make a document about how this process would take place. It's a way all we are in the same page.

https://drive.google.com/drive/folders/0B7N5pSnWAO88S0lmQUpiU0laWWM

Copy link
Member

@juanpicado juanpicado left a comment

Choose a reason for hiding this comment

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

Pretty neat migration 👏 , please check my comments.

@juanpicado juanpicado merged commit f84fd79 into master Oct 3, 2019
@delete-merged-branch delete-merged-branch bot deleted the refactor/116_react_hooks_detail_container branch October 3, 2019 16:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants