-
Notifications
You must be signed in to change notification settings - Fork 80
Feat: added "Fund this package" button #375
Conversation
Codecov Report
@@ Coverage Diff @@
## master #375 +/- ##
==========================================
- Coverage 88.26% 87.77% -0.49%
==========================================
Files 136 137 +1
Lines 946 949 +3
Branches 191 197 +6
==========================================
- Hits 835 833 -2
- Misses 98 103 +5
Partials 13 13
|
@priscilawebdev it has conflicts .. |
|
||
/* eslint-disable react/jsx-no-bind */ | ||
const DetailSidebarFundButton: React.FC<Props> = ({ to }) => { | ||
const preventDefault = (event: MouseEvent<HTMLButtonElement>) => event.preventDefault(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The user clicks the button and then what happens? .... We should open the target in a new tab.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We must also update @verdaccio/types
to be able to have
"funding": {
"type": "opencollective",
"url": "https://opencollective.com/verdaccio"
},
in https://github.com/verdaccio/monorepo/blob/master/core/types/index.d.ts#L132 as optional type. Then can be used here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can find all the rules about that field here https://github.com/npm/rfcs/pull/54/files#diff-1bc199539d172d3211c84688a617e18cR30
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@juanpicado Thanks for your feedback :)
The component StyledLink has the prop "external". When this prop is set to true, the anchor element will have target="_blank" .😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so is it configurable from the verdaccio config file ?
I would make it default to new tab (_blank)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks nice 👍
Thanks for your PR, the @verdaccio/ui package will be accessible from here for testing purposes:
|
The best would be to have a PR with the whole thing instead spread the feature in multiple PR, this is helpful when we need to track issues or when features were introduced, thus the changelog file looks cleaner and anyone can identify the exact version where a feat/bug was introduced. |
Smaller commits / PRs make bisecting easier (I know, we squash them by default). |
Type: Feat
The following has been addressed in the PR: #361
Description: Introduced "Fund this package" button.