search

SublimeText / PackageDev

Tools to ease the creation of snippets, syntax definitions, etc. for Sublime Text.
MIT License
436 stars 83 forks source link

Add some padding to edit settings phantoms #137

Closed deathaxe closed 7 years ago

deathaxe commented 7 years ago

The padding adds some spacing to the left and right of the edit symbol to make the button look better. The symbol is rendered centered. 0.2rem should add a dpi/font-size independent relative spacing which grows with the text.

FichteFoll commented 7 years ago

Can you provide before and after screenshots and the theme & color scheme you use?

For reference, it looks like this for me, which I consider perfectly fine:

img https://github.com/SublimeText/PackageDev/wiki/Settings#quick-edit-icons

deathaxe commented 7 years ago

before

screenshot_before

after

screenshot

Using a modified version of AYU dark color scheme together with Adaptive Theme, but this shouldn't matter as padding and borders are controlled by the stylesheet being used.

Maybe the chosen font and/or OS may make a difference.

FichteFoll commented 7 years ago

I see. So, the reason why it doesn't bother me is because my phantoms use the same background color as the color scheme and phantoms have some padding around them by default. With the change, padding would effectively double because this "box" didn't exist for me.

Would changing the phantom's background color to the color scheme's work for you or do you have an argument against this?

Edit: popupCss in the specific theme (doesn't provide phantomCss):

https://github.com/dempfi/ayu/blob/714c9bbeba98551b9042b4df508b9a141159f878/ayu-dark.tmTheme#L45-L50

deathaxe commented 7 years ago

Removing the background would be an appropriate alternative.

deathaxe commented 7 years ago

Edit: popupCss in the specific theme (doesn't provide phantomCss):

You can not assume all color schemes do!

Therefore PackageDev needs to ensure either correct padding is added in case phantoms are rendered with a different background or it needs to ensure no background is rendered for the added phantoms at all.

FichteFoll commented 7 years ago

it needs to ensure no background is rendered for the added phantoms at all

This is what I decided on. In addition to what you did in your last commit, I also target html, because color schemes may have additional padding on the html tag, making its background color visible even if I override it for the body. Since I set the padding to 0 with that block as well, I don't think we have anything to fear anymore.

deathaxe commented 7 years ago

Just saw the PR was closed but missed the commit.