-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feature: custom repo buttons #15532
feature: custom repo buttons #15532
Conversation
which is similar to Sponsor in github, and user can use it to do other things they like also :) limit the buttons number to 3, because too many buttons will break header ui. Signed-off-by: a1012112796 <1012112796@qq.com>
Codecov Report
@@ Coverage Diff @@
## master #15532 +/- ##
==========================================
- Coverage 43.93% 43.90% -0.03%
==========================================
Files 678 678
Lines 81742 81795 +53
==========================================
+ Hits 35911 35915 +4
- Misses 39980 40021 +41
- Partials 5851 5859 +8
Continue to review full report at Codecov.
|
|
||
const $detailModal = $('#detail-modal'); | ||
|
||
$('.show-repo-button-content').on('click', function () { |
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.
I think this would be a good case for @silverwind's jsx PR, as I am concerned with the below line of injecting HTML and it creating an XSS issue.
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.
Probably not suitable. JSX is when you want to create a DOM structure completely in JS but in this case, it's some pre-rendered Markdown from the server. Generally I don't like putting HTML into data
attributes, maybe it could be rendered directly but hidden via CSS, thought if this is just a dialog, that may already work without any JS involved.
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.
@silverwind How about current way? Thanks
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.
Better, thanks.
In you screenshot, the buttons are missing their left border. Something funky going on with fomantic styles? Oh, and please match button height to the other buttons. |
which is similar to Sponsor in github, and user can use it to do other things they like also :)
limit the buttons number to 3, because too many buttons will break header ui.
example view:
TODO: