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

Apply hint.background and predictive.background colors from the theme #17969

Closed

Conversation

maxdeviant
Copy link
Member

@maxdeviant maxdeviant commented Sep 17, 2024

This PR updates the editor to respect the hint.background and predictive.background colors from the theme.

Before After
Screenshot 2024-09-17 at 4 21 53 PM Screenshot 2024-09-17 at 4 21 43 PM

Closes #17392.

Release Notes:

  • Updated the editor to respect the hint.background and predictive.background colors from the theme.

Co-authored-by: Albert Marashi <albert@lumina.earth>
@maxdeviant maxdeviant self-assigned this Sep 17, 2024
@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Sep 17, 2024
@zed-industries-bot
Copy link

Messages
📖

This PR includes links to the following GitHub Issues: #17392
If this PR aims to close an issue, please include a Closes #ISSUE line at the top of the PR body.

Generated by 🚫 dangerJS against 029d42b

@maxdeviant
Copy link
Member Author

I'm not sure we want to actually proceed with this.

There are a lot of themes—both built-in and in the extension store—that set the background colors, and I think the end result looks quite bad as-is.

@AlbertMarashi
Copy link
Contributor

AlbertMarashi commented Sep 18, 2024

@maxdeviant in regards to the built-in themes, we have two options

  1. Go through each theme and select a good predictive background highlight (happy to do this)
  2. Set all of the colors to #0000 for transparent (this will be effectively a "noop" change)

In regards to extension store themes, unfortunately I think its just a band-aid we have to pull off... Thankfully the fixes/workarounds would be pretty straight forward for both users & theme developers.

In my custom theme, I set any unused colors to #F00 (red) so that I would know to update them if Zed was updated.

@maxdeviant
Copy link
Member Author

@maxdeviant in regards to the built-in themes, we have two options

  1. Go through each theme and select a good predictive background highlight (happy to do this)
  2. Set all of the colors to #0000 for transparent (this will be effectively a "noop" change)

In regards to extension store themes, unfortunately I think its just a band-aid we have to pull off... Thankfully the fixes/workarounds would be pretty straight forward for both users & theme developers.

In my custom theme, I set any unused colors to #F00 (red) so that I would know to update them if Zed was updated.

An issue I have is that even with appropriate colors selected, I still think the end result looks bad with the background in place.

@maxdeviant
Copy link
Member Author

I'm moving the inlay hints portion of this to a separate PR that puts the rendering of backgrounds behind a setting (that defaults to false): #18010

I haven't yet assessed the state of the world for inline completions.

@maxdeviant maxdeviant closed this Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hint/predictive theme stylings have no effect
3 participants