Don't cover input field with file preference browse button

[w-pearson]

What it does

Fixes #12582. The button is now next to the input field instead of on top of it, and thus the button does not cover the end of the input field for long paths.

How to test

Enter a long path in the Windows Terminal preference in the preferences screen. It should be possible to scroll over the input field to see everything.

Before:

After:

Review checklist

• As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Done apart from updating the changelog since the PR that first added the browse button (#10766) didn't update it either; I'm not sure if it's needed in this case.

Reminder for reviewers

• As a reviewer, I agree to behave in accordance with the review guidelines

[msujew]

Thank you @w-pearson for your contribution, that already looks quite good.

I think it makes sense though for the button to align with the input box next to it. Are you fine with performing this change as well? I imagine something like this:

[vince-fugnitto]

As @msujew commented and what I displayed in #12582 (comment), I think the button should be attached to the input, and should also be the same size as the input for a better look-and-feel.

[w-pearson]

Done:

I'm not 100% sure why it needs 3 extra pixels of padding compared to the default 4 above and 4 below on buttons instead of just 2 extra pixels (for the border on the input field that isn't present on the buttons). But this looks correct (and the text in the input field is lined up vertically with the text in the button, at least for electron).

[martin-fleck-at]

@msujew Do you have some time to look at this again? Otherwise I could have a look at it since I did the original implementation.

[vince-fugnitto]

The changes look much better to me compared to the behavior on master, thanks for the fix!
I'll give the chance to others to review as well, but it should be merged for the next release 👍

[msujew]

I've noticed there's a small rendering issue on FireFox (it looks fine on Chrome), where the button is actually 27px in height. Maybe we should just fix it to be something like this instead of playing around with paddings?

Suggested change

	padding-bottom: 6px;
	padding-top: 5px;
	height: 26px;

[w-pearson]

I can't seem to get the "Windows Exec" preference to show up when running in Firefox (with yarn browser start); I previously tested with Electron. How are you doing it?

Using exact pixel values seems like it would lead to more problems later (unless the height of the text field is also set in pixels, which it doesn't seem to be). I don't see any obvious way to make it automatically use the exact same amount of vertical space/all available vertical space (height: 100% didn't do it).

[msujew]

I've just modified a preference to use the typeDetails: { isFilepath: true } property. See here. In my case, I've used the window.title preference.

unless the height of the text field is also set in pixels, which it doesn't seem to be

It's not, but it's using line-height which is almost the same for the purpose of size calculation as plainly using height.

[w-pearson]

I used calc(var(--theia-content-line-height) + 4px) (which is equivalent to 26px). That seems to give correct results with firefox and electron-chromium.

[martin-fleck-at]

@msujew Is the proposed solution fine with you? If so, we could get this PR merged.

[w-pearson]

As a side question: should I be amending and then force-pushing? Or do you want individual fixup commits like I've been doing?

[vince-fugnitto]

As a side question: should I be amending and then force-pushing? Or do you want individual fixup commits like I've been doing?

@w-pearson in general, when addressing review feedback we should add new commits as per the guidelines for fixups. In the end we will "squash and merge" anyways unless we want to preserve the commit history for bigger changes (that wanted commits to be separated on purpose).

[msujew]

Looks good to me 👍