Skip to content
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

fix: HTML report tinkering #1561

Merged
merged 11 commits into from
Feb 5, 2025
Merged

Conversation

another-rex
Copy link
Collaborator

@another-rex another-rex commented Feb 3, 2025

Made a series of changes to resolve the issues identified in #1528

Hosted an example here: https://another-rex.github.io/TestPages/Vulnerability%20Scan%20Report.html

To make it easier to see the changes, when reviewing, use this link:
https://github.com/google/osv-scanner/pull/1561/files/bd8d5211e77612b1a0a68a4b00db1d40535fe400..8223593ef84cda34da84a57aecd12d3258ea1463
which select the diffs Excluding the first commit (use shift to select multiple commits). That moves the files around which breaks all of git's diffing. No change other than moving the files and reindenting is done in that first commit.

HTML:

  • Move to actual .js and .css file rather than .html files.
  • Alias and groupid tooltips now put each ID on a new line.
  • Can now click on the entire filter box to change it, not just on the text part.

CSS:

  • Remove max-height in the inner tables, this was making it impossible to have tooltips that escape the container (at least I haven't figured out how to have both).
  • Tooltip box sizing is now dynamic with max-width
  • Tooltips now display upwards instead of downwards
  • Highlight source path better
  • Minor refactor to how the search box is laid out
  • Remove unused css lines.
  • Make iframe bg color black instead of white to avoid flash banging people.

JS:

  • Remove all style edits in javascript, state changes are made with classes now. (TIL classList.toggle() function)
  • Basic linter pass (e.g. use const on variables, define all variables...etc)
  • Run showAllVulns() function at page load.

@another-rex another-rex requested a review from hogo6002 February 3, 2025 02:06
@codecov-commenter
Copy link

codecov-commenter commented Feb 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.29%. Comparing base (a5d0e2b) to head (2a34bbc).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1561      +/-   ##
==========================================
- Coverage   69.31%   69.29%   -0.02%     
==========================================
  Files         200      200              
  Lines       19038    19046       +8     
==========================================
+ Hits        13196    13198       +2     
- Misses       5135     5140       +5     
- Partials      707      708       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hogo6002
Copy link
Contributor

hogo6002 commented Feb 3, 2025

Thanks! can you host one sample HTML on Github

@another-rex
Copy link
Collaborator Author

Copy link
Contributor

@hogo6002 hogo6002 left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning up the JS file with best practice!

Copy link
Contributor

@hogo6002 hogo6002 left a comment

Choose a reason for hiding this comment

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

Thanks!

@another-rex another-rex merged commit 5b166c1 into google:main Feb 5, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants