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.

Sign up for GitHub

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

Jump to bottom

[1.17] Launch Params in SUI needs a little cleanup #14558

Closed
carlos-zamora opened this issue Dec 14, 2022 · 6 comments · Fixed by #14569
Closed

[1.17] Launch Params in SUI needs a little cleanup #14558

carlos-zamora opened this issue Dec 14, 2022 · 6 comments · Fixed by #14569
Assignees
@carlos-zamora
Labels
Area-Settings UI Anything specific to the SUI Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal v1.17

Comments

@carlos-zamora
Copy link
Member

carlos-zamora commented Dec 14, 2022 •
edited by zadjii-msft
Loading

Bug 1: Launch Params resizes as you type X/Y val

Duped to #13495 image

Then when you click off of it, it looks like the num box buttons appear over the text? Weird.
image

Bug 2: rework Launch Params preview

I still feel like "Default,Default" should be something like…

"Let system position window" or "system position" or whatever

Should "Use system default" be renamed to "Let system position window"
@carlos-zamora carlos-zamora added Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. Area-Settings UI Anything specific to the SUI labels Dec 14, 2022
@carlos-zamora carlos-zamora added this to the Terminal v1.17 milestone Dec 14, 2022
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Dec 14, 2022
@carlos-zamora carlos-zamora removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Dec 14, 2022
@carlos-zamora carlos-zamora changed the title [1.17] Launch Params resizes as you type X/Y val [1.17] Launch Params in SUI needs a little cleanup Dec 15, 2022
@ianjoneill
Copy link
Contributor

I think bug 1 may be a dupe of #13495 - the history size input does exactly the same thing.

@zadjii-msft
Copy link
Member

I think bug 1 may be a dupe of #13495

Yes it is

Should "Use system default" be renamed to "Let system position window"

absolutely yes

@carlos-zamora
Copy link
Member Author

@zadjii-msft Hmm... Since #13495 is tracking upgrading to WinUI 2.8 (which would fix the bug, of course, but it's a lot of extra work). How do we feel about changing the SpinButtonPlacementMode from Inline to Compact? I assume that fixes the problem and it would be super simple.

@carlos-zamora carlos-zamora self-assigned this Dec 15, 2022
@zadjii-msft
Copy link
Member

How do we feel about changing the SpinButtonPlacementMode from Inline to Compact? I assume that fixes the problem and it would be super simple.

I mean, would it? I'm honestly fine leaving that half of the bug as is (and waiting for eventually figuring out winui 2.8), it only happens with huge numbers anyways, yea?

@carlos-zamora carlos-zamora mentioned this issue Dec 15, 2022
Merged
@ghost ghost added the In-PR This issue has a related PR label Dec 15, 2022
@carlos-zamora
Copy link
Member Author

How do we feel about changing the SpinButtonPlacementMode from Inline to Compact? I assume that fixes the problem and it would be super simple.

I mean, would it? I'm honestly fine leaving that half of the bug as is (and waiting for eventually figuring out winui 2.8), it only happens with huge numbers anyways, yea?

I did the thing: #14569

@ghost ghost closed this as completed in #14569 Dec 16, 2022
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Dec 16, 2022
ghost pushed a commit that referenced this issue Dec 16, 2022
@carlos-zamora
[SUI] Polish Launch Parameters setting controls (#14569)
a3c7bc3 
## Summary of the Pull Request
Performs some cleanup in the Settings UI for the Launch Parameters settings:
1. Updates all `NumberBox` controls in the Settings UI to have a `Compact` `SpinButtonPlacementMode` instead of an inline one. This alleviates the XAML bug where the spin button would appear over the number box's input value.
2. Fixes an issue where a long X/Y value would resize the settings controls weirdly. This was fixed by introducing a `Grid` inside the main grid and applying a width to the number boxes.
3. Rename "Use system default" checkbox to be more clear. Propagate the new localized string into expander preview.


Closes #14558
@ghost
Copy link

ghost commented Jan 24, 2023

🎉This issue was addressed in #14569, which has now been successfully released as Windows Terminal Preview v1.17.1023.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@carlos-zamora carlos-zamora

Labels
Area-Settings UI Anything specific to the SUI Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal v1.17
Development

Successfully merging a pull request may close this issue.

[SUI] Polish Launch Parameters setting controls microsoft/terminal
3 participants
@ianjoneill @carlos-zamora @zadjii-msft