Skip to content

Conversation

@notlmn
Copy link
Contributor

@notlmn notlmn commented Jun 26, 2020

Fixes: #2991

Uses clipped SVG background images for spaces and tabs, this makes it more flexible for text wrapping, and any other future characters (#2150).

Test


This PR is still in the works, as SVG content used are static paths, which should ideally be text nodes that inherit font from DOM (Figma still flattens all text nodes while exporting).

I tried to match the existing styles (and size) as much as possible, but we might have to reconsider that anyway (i.e. using actual text).


Edit: Updated GIFs

@fregante
Copy link
Member

Great idea if it works reliably. However I’d preserve the previous opacity, this one isn’t very visible.

@notlmn
Copy link
Contributor Author

notlmn commented Jun 26, 2020

That's just the very thin tab character from Inconsolata flattened to <path> in SVG.

I'm still working on using <text> nodes here to get proper font rendering (if possible). Wanted to put this PR out for sharing the idea.

(As mentioned Figma flattens all text nodes to <path>s in SVG, and I'm still afraid to use Inkscape 😅, will have to find some other vector edit tool)

@fregante
Copy link
Member

You can make the SVG from scratch, it’s just 2 tags (text tag inside svg tag)

@notlmn
Copy link
Contributor Author

notlmn commented Jun 29, 2020

Looks like there is no way for SVG backgrounds (either using data: or http:) would inherit properties like font and line-height from parent DOM. Even if that were to happen, it would be very complex to handle, especially baseline was very tricky with background-size.

Instead I'd propose the use of static path elements, this way the backgrounds says consistent on any platform regardless of the font.

Open to other ideas as well.

@fregante
Copy link
Member

That’s right 👍 let’s match the current color though

@notlmn
Copy link
Contributor Author

notlmn commented Jun 30, 2020

That’s right +1 let’s match the current color though

Done!

@notlmn notlmn marked this pull request as ready for review June 30, 2020 04:50
@fregante fregante added the bug label Jul 1, 2020
@fregante fregante self-requested a review July 11, 2020 21:20
@fregante fregante changed the title Use clipped backgrounds for showing whitespaces Fix display of wrapped whitespace in show-whitespace Jul 16, 2020
@fregante fregante merged commit bb5b3e9 into master Jul 16, 2020
@fregante fregante deleted the whitespace-backgrounds branch July 16, 2020 09:36
@fregante
Copy link
Member

This is a clever solution and it appears to work. Let's see if it causes any issues in the real world, I'll publish this today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

show-whitespace isn't wrapped to next line properly

2 participants