-
-
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
Issue/PR Context Popups #9822
Issue/PR Context Popups #9822
Conversation
Signed-off-by: jolheiser <john.olheiser@gmail.com>
Signed-off-by: jolheiser <john.olheiser@gmail.com>
Signed-off-by: jolheiser <john.olheiser@gmail.com>
Signed-off-by: jolheiser <john.olheiser@gmail.com>
Signed-off-by: jolheiser <john.olheiser@gmail.com>
Signed-off-by: jolheiser <john.olheiser@gmail.com>
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.
This looks great. If this could be used for the PR list and show information like number of approvals, blocking issues or mergeability, we could tick a couple of boxes in #8659.
I'm approving nonetheless because that can be done in a 2nd PR.
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 text inside the popup is almost unreadable on dark mode.
Dark style needs some fixes. A less intense border and the triangle pointer. I'd suggest not styling this particular popup, but |
Signed-off-by: jolheiser <john.olheiser@gmail.com>
@silverwind I updated the OP screenshots, do they look better now? @bagasme I tried to make the color lighter, but imho it looks better as-is. This is a little too bright. For reference, the original color is the same as |
Signed-off-by: jolheiser <john.olheiser@gmail.com>
Signed-off-by: jolheiser <john.olheiser@gmail.com>
If you can move the related js codes on a spereate js file, that will be perfect!!! |
@jolheiser much better, thanks. I'd maybe go a shade darker even on the border, but no biggie. @lunny good idea. I'd suggest moving the code to export function initContextPopups() {
const refIssues = $('.ref-issue');
if (!refIssues.length) return;
// your code here
} and load via import { initContextPopups } from './features/contextPopups.js'; |
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.
nice PS: for js stuff i point to silverwind ....
Codecov Report
@@ Coverage Diff @@
## master #9822 +/- ##
==========================================
- Coverage 42.27% 42.24% -0.04%
==========================================
Files 605 605
Lines 79251 79262 +11
==========================================
- Hits 33505 33484 -21
- Misses 41616 41644 +28
- Partials 4130 4134 +4
Continue to review full report at Codecov.
|
Signed-off-by: jolheiser <john.olheiser@gmail.com>
Signed-off-by: jolheiser <john.olheiser@gmail.com>
@silverwind Done. Had to do it slightly different to avoid being yelled at by the linter. |
Co-Authored-By: silverwind <me@silverwind.io>
Just came across this new feature in V12. Really polishes up the interface. Great work @jolheiser (and company ;) ) |
Context popup when hovering over linked issue/PRs.
There are a few things missing because they weren't easily available, but overall I think this is enough to warrant a PR.