Svelte: add copy button and reduce spacing

[camdencheek]

This adds a copy button to the file path in the file view and reduces spacing between path elements.

Mostly fixes SRCH-140

Notably:

• It does not fix the "browser adds spaces between elements when copied" issue. For the life of me, I could not figure out how to remove the spaces. It seems like there should be a way, but it feels less important if there is a copy button
• It does not extend the h1 element to fill the space, but rather shows the copy button when hovering over the whole header. This is not ideal, but the "compress actions" logic depends on the actions section taking up as much space as is available, so we can't easily change that. Will come back to this after we've implemented the new header layout, but I think this is good enough for now

Test Plan

Added playwright tests for copy button and copying text from the file header

screenshot-2024-05-22_16-14-03.mp4

[camdencheek]

Changed this to display: flex; so we can control the spacing between elements.

[taiyab]

Most importantly, does this feel good?

[camdencheek]

does this feel good?

The existence of a copy button feels good. It's way easier to use than select+copy. The fact that it shows up when you hover the actions feels a little weird, but is not really noticeable or problematic. And I think there's a good chance that will be (much more easily) fixable with the new designs for the header.

[camdencheek]

I like the reduced spacing as well. The increased density is helpful for that space

[fkling]

I'll take a look tomorrow.

For the life of me, I could not figure out how to remove the spaces.

Have you tried removing the space in line 52? Although I'd think that it would also be ignored with display: flex, but not sure. I can try some things out tomorrow too. Copy button aside I still think selecting and copying text should work as expected.

[camdencheek]

Have you tried removing the space in line 52?

I have. Even with no whitespace in the final, rendered DOM, copying still adds spaces. I think it might actually be display: flex that's the issue. I also tried display: inline-flex, but with no luck. Would love to see what you find 🙂

[vovakulikov]

@camdencheek Copied value is affected by display rule of the elements from which we're getting values for copy buffer. (sometimes, parents can change these display rules implicitly, like the display: flex changes display on the child elements on flex-item. Also HTML is important as well, see examples below.

Inline elements with space in HTML

So for example this

<div>
   <span>Hello</span>
   <span>World</span>
</div>

will produce "Hello World" with space between words, because there is a new line in HTML between inline elements,

Inline elements with no space

However this

<div>
   <span>Hello</span><span>World</span>
</div>

will produce "HelloWorld" since there is no separator in HTML between inline elements.

Non inline elements

<div>
   <div>Hello</div>
   <div>World</div>
</div>

This will produce

Hello 
Worlds

regardless of separators in HTML, so this

<div>
   <div>Hello</div><div>World</div>
</div>

also produces just two lines of text

Hello 
Worlds

since text elements are non inline elements

Non-inline elements because of the parent layout

<div styles="display:flex">
   <span>Hello</span><span>World</span>
</div>

will produce "Hello world" since text elements again are non inline cause of their parent.

[camdencheek]

Thanks for the info @vovakulikov! It seems like I was running into a combination of complicated CSS behavior and also weird svelte behavior. It's really ugly to remove the spaces from this component's markup, and I still can't figure out how to ignore the inline icon, which still adds a space.

The best I could come up with was to add a copy event listener that strips the newlines. It feels hacky, but it works more consistently than I expect any DOM massaging will.

If anyone wants to try to nerd snipe a listener-free alternative, I'd love to see it, but I think this might be the best we're gonna get 😄

[camdencheek]

Okay, also added playwright tests that cover this since it would be a super easy thing to regress on

[fkling]

Thank you! Indeed it looks like setting display affects the value that is copied, which I find surprising but I guess we can't do anything about it.

[fkling]

selection should always be defined, since you do ?? '' above. So I think you can drop the ? here:

Suggested change

	event.clipboardData?.setData('text/plain', selection?.toString()?.replaceAll(/\n?\/\n?/g, '/'))
	event.clipboardData?.setData('text/plain', selection.toString().replaceAll(/\n?\/\n?/g, '/'))