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

Add modified background to dracula popup #1434

Merged
merged 1 commit into from
Jan 16, 2022

Conversation

stuarth
Copy link
Contributor

@stuarth stuarth commented Jan 4, 2022

Modifies the background color in the dracula theme so popup boundaries are clear.

Before

Screen Shot 2022-01-03 at 9 54 33 PM

After

Screen Shot 2022-01-03 at 9 53 29 PM

Still a bit low contrast, so happy to iterate on color, but vastly better having it distinguished. cc @loewenheim

@twe4ked
Copy link

twe4ked commented Jan 4, 2022

I agree this is better than not having it, but FWIW I couldn't see the difference when I first looked at the screenshots. Would any other colors make sense?

@loewenheim
Copy link
Contributor

This is a definite improvement. Personally I'd like a border around the popup, but I don't know if that's possible.

@Omnikar
Copy link
Contributor

Omnikar commented Jan 4, 2022

This is a definite improvement. Personally I'd like a border around the popup, but I don't know if that's possible.

We might be able to draw borders using these characters for edges, not sure about corners though.

▇   Top edge (swap bg and fg colors)
▎   Left edge
▋   Right edge (swap bg and fg colors)
▁   Bottom edge

@archseer
Copy link
Member

archseer commented Jan 6, 2022

We already have box drawing in helix-tui via Block and it supports a bunch of styles:

pub enum BorderType {
Plain,
Rounded,
Double,
Thick,
}

I avoided adding a border to popups though to reduce the size of the popup since the border would add another character of padding on each side.

@Omnikar
Copy link
Contributor

Omnikar commented Jan 6, 2022

I avoided adding a border to popups though to reduce the size of the popup since the border would add another character of padding on each side.

But it appears that currently there are spaces occupying the edges of the popups, couldn't those just be replaced with the box drawing characters?

@archseer
Copy link
Member

archseer commented Jan 6, 2022

We could do that and make it configurable. Generally we have a 1 char padding between the border and content on other windows (for example the auto info popup, picker, etc)

@archseer
Copy link
Member

archseer commented Jan 6, 2022

popup.border-type = "none"/"plain"/"rounded"

@archseer
Copy link
Member

Merging this for now :) We can open another issue for borders

@archseer archseer merged commit 9da0aba into helix-editor:master Jan 16, 2022
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.

5 participants