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

fix: remove prevent default and use react context#411

Merged
juanpicado merged 3 commits intomasterfrom
fix-410
Jan 8, 2020
Merged

fix: remove prevent default and use react context#411
juanpicado merged 3 commits intomasterfrom
fix-410

Conversation

@juanpicado
Copy link
Copy Markdown
Member

@juanpicado juanpicado commented Jan 4, 2020

Type: bug

The following has been addressed in the PR:

  • There is a related issue? yes
  • Unit or Functional tests are included in the PR

Description:

  • fix the bug
  • use the context as the other components in the same parent ( for consistence )
  • add a new test

Resolves #410

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 4, 2020

Codecov Report

Merging #411 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master     #411   +/-   ##
=======================================
  Coverage   88.38%   88.38%           
=======================================
  Files         137      137           
  Lines         956      956           
  Branches      198      211   +13     
=======================================
  Hits          845      845           
+ Misses         99       98    -1     
- Partials       12       13    +1
Impacted Files Coverage Δ
...mponents/DetailSidebar/DetailSidebarFundButton.tsx 100% <100%> (ø) ⬆️
src/components/Icon/styles.ts 88.88% <0%> (ø) ⬆️

@juanpicado juanpicado marked this pull request as ready for review January 5, 2020 09:05
@juanpicado juanpicado requested a review from DanielRuf January 5, 2020 09:05
@juanpicado juanpicado added the bug label Jan 5, 2020
@juanpicado juanpicado requested a review from DanielRuf January 5, 2020 10:49
@verdacciobot
Copy link
Copy Markdown

Thanks for your PR, the @verdaccio/ui package will be accessible from here for testing purposes:

npm install @verdaccio/[email protected] --registry https://registry.verdaccio.org

@juanpicado
Copy link
Copy Markdown
Member Author

@DanielRuf reminder :)

@DanielRuf
Copy link
Copy Markdown
Contributor

Testing now.

Copy link
Copy Markdown
Contributor

@DanielRuf DanielRuf left a comment

Choose a reason for hiding this comment

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

LGTM

I have added the funding field to partials/storage/jquery/package.json and clicked on the button:

		"3.3.1": {
			"funding": {
				"url": "https://example.com"
			},
			"name": "jquery",
			"title": "jQuery",

grafik

And it worked, I was redirected to example.com.

@juanpicado juanpicado merged commit 6bd38b8 into master Jan 8, 2020
@delete-merged-branch delete-merged-branch bot deleted the fix-410 branch January 8, 2020 11:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fund button is not working

3 participants