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

feat: load html for oss in ls [IDE-285] #570

Merged
merged 1 commit into from
Jul 12, 2024

Conversation

teodora-sandu
Copy link
Contributor

@teodora-sandu teodora-sandu commented Jul 10, 2024

Description

Adds some styling so that the new HTML panel and the native panel look more or less the same. The only noticeable differences are:

  • The text in the Detailed paths changed from Fix to Remediation
  • The font for the code, which now matches the one chosen by the customer
  • The labels don't have a colon anymore, which matches what we see in VSCode too (could add the colons back if needed via CSS)

Refactors the code a bit too so that we can re-use more of it for both Snyk Code and Snyk OSS.

Needs snyk/cli#5356 for testing.

Checklist

  • Tests added and all succeed
  • Linted
  • CHANGELOG.md updated
  • README.md updated, if user-facing

Screenshots / GIFs

Native:
goal 1
goal 2

HTML:
Screenshot 2024-07-10 at 13 24 10
Screenshot 2024-07-10 at 13 24 21

@teodora-sandu teodora-sandu force-pushed the feat/html-oss-intellij branch 2 times, most recently from bddd3b1 to 154f250 Compare July 10, 2024 13:30
@teodora-sandu teodora-sandu marked this pull request as ready for review July 10, 2024 13:48
@teodora-sandu teodora-sandu requested a review from a team as a code owner July 10, 2024 13:48
fun generate(jbCefBrowser: JBCefBrowserBase): CefLoadHandlerAdapter {
val isDarkTheme = EditorColorsManager.getInstance().isDarkEditor
val isHighContrast =
EditorColorsManager.getInstance().globalScheme.name.contains("High contrast", ignoreCase = true)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wondering: should we also set the other font? It looks like the font is different in "native" and "html", and that maybe is due to not taking the theme font?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could, but what would be the point of it? Is it to make the change more gradual once we enable this feature?

@teodora-sandu teodora-sandu force-pushed the feat/html-oss-intellij branch from 154f250 to e522ac8 Compare July 10, 2024 15:44
@teodora-sandu teodora-sandu force-pushed the feat/html-oss-intellij branch from e522ac8 to 00392c2 Compare July 10, 2024 15:47
@teodora-sandu teodora-sandu merged commit 849f18b into master Jul 12, 2024
9 checks passed
@teodora-sandu teodora-sandu deleted the feat/html-oss-intellij branch July 12, 2024 08:55
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.

2 participants