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

ui: Add background color property to label #22109

Closed

Conversation

helgemahrt
Copy link
Contributor

@helgemahrt helgemahrt commented Dec 16, 2024

For issue #14093 I need to be able to specify a background color on the label used for the vim mode indicator.

Release Notes:

  • N/A

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Dec 16, 2024
Copy link
Member

@maxdeviant maxdeviant left a comment

Choose a reason for hiding this comment

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

@danilo-leal raised a good point: is there a reason this needs to be on the label instead of just setting a background color for the containing element?

@maxdeviant maxdeviant self-assigned this Dec 16, 2024
@helgemahrt
Copy link
Contributor Author

helgemahrt commented Dec 16, 2024

@danilo-leal raised a good point: is there a reason this needs to be on the label instead of just setting a background color for the containing element?

Let me preface this with saying that I'm still new to this code base. :) From what I saw, the vim status indicator is a label sitting directly on the status bar and I didn't want to change the background color of the whole status bar: https://github.com/zed-industries/zed/blob/main/crates/vim/src/mode_indicator.rs#L111 Is there another containing element? I saw that Label internally has a Div, but setting bg on the Div did not have any effect.

@maxdeviant
Copy link
Member

@danilo-leal raised a good point: is there a reason this needs to be on the label instead of just setting a background color for the containing element?

Let me preface this with saying that I'm still new to this code base. :) From what I saw, the vim status indicator is a label sitting directly on the status bar and I didn't want to change the background color of the whole status bar: https://github.com/zed-industries/zed/blob/main/crates/vim/src/mode_indicator.rs#L111 Is there another containing element? I saw that Label internally has a Div, but setting bg on the Div did not have any effect.

We could add a wrapping div in the ModeIndicator if we need to put a background color on it.

@maxdeviant
Copy link
Member

I'm going to close this PR out, as it seems we can achieve the results we want without modifying Label.

@maxdeviant maxdeviant closed this Dec 16, 2024
@helgemahrt helgemahrt deleted the label-background-color branch December 17, 2024 06:33
@helgemahrt
Copy link
Contributor Author

Just achieved the same by using a div. Thanks, I learned something today! :)

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.

2 participants