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

Header Component - Replaced class by func. comp #142

Merged
merged 17 commits into from Oct 8, 2019

Conversation

priscilawebdev
Copy link
Contributor

@priscilawebdev priscilawebdev commented Oct 1, 2019

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?

Yes.

Description:

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

@codecov-io
Copy link

codecov-io commented Oct 1, 2019

Codecov Report

Merging #142 into master will increase coverage by 1.28%.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #142      +/-   ##
==========================================
+ Coverage   83.77%   85.06%   +1.28%     
==========================================
  Files          99      107       +8     
  Lines         937      924      -13     
  Branches      161      160       -1     
==========================================
+ Hits          785      786       +1     
+ Misses        138      124      -14     
  Partials       14       14
Impacted Files Coverage Δ
src/components/Header/HeaderGreetings.tsx 100% <100%> (ø)
src/components/Header/HeaderMenu.tsx 100% <100%> (ø)
src/components/Header/HeaderInfoDialog.tsx 100% <100%> (ø)
src/components/Header/HeaderLeft.tsx 100% <100%> (ø)
src/components/Header/HeaderToolTip.tsx 100% <100%> (ø)
...mponents/RegistryInfoDialog/RegistryInfoDialog.tsx 100% <100%> (+33.33%) ⬆️
src/components/Header/HeaderLogo.tsx 75% <75%> (ø)
src/components/Header/Header.tsx 75% <75%> (-14.48%) ⬇️
src/components/Header/HeaderToolTipIcon.tsx 83.33% <83.33%> (ø)
src/components/Header/HeaderRight.tsx 93.33% <93.33%> (ø)
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae73772...5d13173. Read the comment docs.

@priscilawebdev priscilawebdev changed the title Work in Progress: Header Component - Replaced class by func. comp Header Component - Replaced class by func. comp Oct 3, 2019
@juanpicado
Copy link
Member

juanpicado commented Oct 3, 2019

Your last PR conflicts with this one f84fd79#diff-2780c4d0882ad10abae71aa33af34c7b

@juanpicado
Copy link
Member

+1% coverage 🥰 good job 👏🏻

juanpicado
juanpicado previously approved these changes Oct 6, 2019
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.

great 🚀

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.

great 🚀

);

expect(container.firstChild).toMatchSnapshot();
expect(queryByTestId('header--menu-acountcircle')).toBeNull();
Copy link
Member

Choose a reason for hiding this comment

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

Reminder, the test-id is fine for me, but, according the guidelines, should be the 3rd option.

https://testing-library.com/docs/guide-which-query

@juanpicado juanpicado merged commit d1ce828 into master Oct 8, 2019
@delete-merged-branch delete-merged-branch bot deleted the refactor/130_convert_class_to_func_header branch October 8, 2019 05:47
@aforty
Copy link

aforty commented Dec 31, 2019

This commit had the unintended consequence of breaking the plugin verdaccio-github-oauth-ui which was relying on id="#header--button-login" and id="#header--button-logout" being set on the Login and Logout buttons in order to override their functions.

Not sure what the fix here is. Add the id's back to modify the plugin to look for the id's in data-testid?

Issue to track is here: n4bb12/verdaccio-github-oauth-ui#39

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants