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.

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

Add a fun new preview text in the SUI, enable the cursor #15363

Merged
merged 6 commits into from
May 16, 2023

Conversation

DHowett
Copy link
Member

@DHowett DHowett commented May 16, 2023

Our existing preview text was not very helpful in learning how different settings impacted the display of text in Terminal.

This new preview text contains:

  • Bold text, which is controlled by intenseTextStyle
  • Colors
  • Emoji
  • A cursor, which overlaps a single character to show inversion behavior

@DHowett
Copy link
Member Author

DHowett commented May 16, 2023

preview1 preview2

@DHowett
Copy link
Member Author

DHowett commented May 16, 2023

I have something even MORE fun, but i was holding it for after this...

@github-actions

This comment has been minimized.

@@ -111,6 +111,8 @@ namespace Microsoft.Terminal.Control
// opacity set by the settings should call this instead.
Double BackgroundOpacity { get; };

Boolean DisplayCursorWhileBlurred;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blur is the opposite of focus, don't at me

Copy link
Member

@lhecker lhecker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not happy about the DisplayCursorWhileBlurred because it's a hack. It would probably make more sense if it was part of the unfocused appearance as a proper setting. It might be worthwhile to note in the .idl that the setting is bodgy/hacky.

src/cascadia/TerminalControl/TermControl.cpp Show resolved Hide resolved
@DHowett
Copy link
Member Author

DHowett commented May 16, 2023

I'm not happy about the DisplayCursorWhileBlurred because it's a hack.

I agree, but I did look at other XAML properties like AllowFocusWhenDisabled and stuff. There is precedent for these weird single-shot properties.

I do think we should fix Unfocused Appearances, but in general doing that will utterly F up the settings UI preview because it is always unfocused... SO WE WILL STILL NEED A HACK 😁

@lhecker
Copy link
Member

lhecker commented May 16, 2023

I do think we should fix Unfocused Appearances, but in general doing that will utterly F up the settings UI preview because it is always unfocused... SO WE WILL STILL NEED A HACK 😁

We could technically just separate out the WinUI and non-WinUI parts from the _GotFocusHandler and _LostFocusHandler and expose the latter as a property. This would allow you to switch between the two appearances at will. I do agree though that we can do something like this at a later time (unless you feel like this would be easy to implement?). As you mentioned, there's a lot of subjective design issues in the area already that we'd probably need to address.

@DHowett DHowett merged commit fbe45fa into main May 16, 2023
@DHowett DHowett deleted the dev/duhowett/preview-text branch May 16, 2023 19:59
DHowett added a commit that referenced this pull request May 16, 2023
Our existing preview text was not very helpful in learning how different
settings impacted the display of text in Terminal.

This new preview text contains:
* Bold text, which is controlled by intenseTextStyle
* Colors
* Emoji
* A cursor, which overlaps a single character to show inversion behavior

(cherry picked from commit fbe45fa)
Service-Card-Id: 89230301
Service-Version: 1.17
DHowett added a commit that referenced this pull request May 16, 2023
Our existing preview text was not very helpful in learning how different
settings impacted the display of text in Terminal.

This new preview text contains:
* Bold text, which is controlled by intenseTextStyle
* Colors
* Emoji
* A cursor, which overlaps a single character to show inversion behavior

(cherry picked from commit fbe45fa)
Service-Card-Id: 89230302
Service-Version: 1.18
zadjii-msft added a commit that referenced this pull request Sep 19, 2023
## Summary of the Pull Request

Resolves the following in #15812 

> - [x] `toggleBroadcastInput` isn't in the default settings
> - [x] The cursors forget to keep blinking if you focus each pane and
then unfocus them
> - [x] They don't stop blinking when you unbroadcast
> - [x] Broadcast border doesn't appear when you make new panes, but
they ARE broadcasted-to!

## References and Relevant Issues
x-ref:
* #2634
* #14393

## Detailed Description of the Pull Request / Additional comments

There was literally no logic in the original PR for starting the cursor
blinking. It's entirely unknowable how that ever worked. This makes it
all much more explicit.

We're taking the hacky `DisplayCursorWhileBlurred` from #15363, and
promoting that to the less-hacky `CursorVisibility`. Broadcast input
mode can use that to force the cursor to be visible always.


The last checkbox in that issue is harder, and I didn't want to further
pollute this delta with the paste plumbing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants