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

use class for diagnostic info instead of hardcoding color #2257

Merged
merged 7 commits into from
May 17, 2023
Merged

Conversation

rchl
Copy link
Member

@rchl rchl commented May 14, 2023

Use .color-muted class instead of hardcoding color when showing diagnostic info.

Changed opacity of .color-muted from 0.5 to 0.6. Should still be fine and is more consistent with other places.

Fixes #2255

@rchl rchl changed the title use .color-muted class for diagnostic info instead of hardcoding color use class for diagnostic info instead of hardcoding color May 14, 2023
@rchl
Copy link
Member Author

rchl commented May 14, 2023

Also we had this existing issue where the "related info" did not have a line break before it in the diagnostic panel.

Before:

Screenshot 2023-05-14 at 22 28 34

After:

Screenshot 2023-05-14 at 22 27 27

The HTML in the quick panel preview seems to be more limited so applying styles from popup.css wouldn't help there. So added extra <br> instead. Hover popup still shows things correctly with extra BR:

Screenshot 2023-05-14 at 22 29 39

Copy link
Member

@predragnikolic predragnikolic left a comment

Choose a reason for hiding this comment

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

OK, with this PR, someone will be able to override the styles to their liking

Tested ✔️

before:
Screenshot from 2023-05-16 00-00-46

after (I've overridden some CSS classes)
Screenshot from 2023-05-15 23-58-30

@jwortmann
Copy link
Member

The screenshot don't look right to me, why is there a linebreak before the link (and also before the closing ) in the second screenshot)? Have you maybe applied a margin/padding to the .color-muted class in your custom rules, which could cause this?

I think instead of having the <span class="color-muted"> tags around each separate token, it would be better to use only a single wrapper tag of this around the whole Code(CodeDescription) expression. This would prevent such oddities as seen above.

@rchl
Copy link
Member Author

rchl commented May 16, 2023

I think the intentions of @predragnikolic were not to achieve something nice looking but just test that overrides work.
We can't prevent people from messing up stuff if they want to.

We could try to have one color-muted wrapper but there is an optional link inside that element so not sure how will it work.

@rchl
Copy link
Member Author

rchl commented May 16, 2023

It seems to look the same when the link is also within the .color-muted class.

Before

Screenshot 2023-05-16 at 08 57 58

After

Screenshot 2023-05-16 at 09 06 36

@predragnikolic
Copy link
Member

The screenshot don't look right to me, why is there a linebreak before the link (and also before the closing ) in the second screenshot)?

There was some custom CSS that caused such behavior :)

I still didn't try to make it pretty, but I just tested that it is possible to target the class name.

Screenshot from 2023-05-16 14-06-10

I think the intentions of @predragnikolic were not to achieve something nice looking but just test that overrides work.

Yes

We can't prevent people from messing up stuff if they want to.

And yes

@predragnikolic
Copy link
Member

Looks good to me,
@jwortmann, wdyt?

@jwortmann
Copy link
Member

Looks good to me too, but only one small nitpick, that I would not include the leading space in the span.color-muted tag.
So I think

-       meta_info = " "
        html = " "

should do it.

@rchl rchl merged commit 302476d into main May 17, 2023
@rchl rchl deleted the diagnostic-class branch May 17, 2023 16:51
rchl added a commit that referenced this pull request May 30, 2023
* main:
  fix "Error rewriting command" warning triggered on startup (#2277)
  Take font style of sighelp active parameter from color scheme (#2279)
  Add argument "include_declaration" to "lsp_symbol_references" (#2275)
  fix crash on checking excluded folders with missing project data (#2276)
  Fix tagged diagnostics flickering on document changes (#2274)
  Cut 1.24.0
  use class for diagnostic info instead of hardcoding color (#2257)
  Fix package storage path in a docstring description (#2256)
  Use regular font style in sighelp popup if already highlighted by color scheme (#2259)
  update note about custom color scheme rule used for diagnostics (#2254)
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.

How can I change the color of this?
3 participants