Skip to content

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

Merged
DHowett merged 6 commits intomainfrom
dev/duhowett/preview-text
May 16, 2023
Merged

Add a fun new preview text in the SUI, enable the cursor#15363
DHowett merged 6 commits intomainfrom
dev/duhowett/preview-text

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...

@DHowett DHowett requested a review from PankajBhojwani May 16, 2023 15:21
@github-actions

This comment has been minimized.

Sign in to view
DHowett
DHowett commented May 16, 2023
View reviewed changes
src/cascadia/TerminalControl/TermControl.idl
// opacity set by the settings should call this instead.
Double BackgroundOpacity { get; };

Boolean DisplayCursorWhileBlurred;
Copy link
Member Author

@DHowett DHowett May 16, 2023

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

lhecker
lhecker approved these changes May 16, 2023
View reviewed changes
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.

@DHowett
Copy link
Member Author

DHowett commented May 16, 2023 •
edited
Loading

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
@DHowett
Add a fun new preview text in the SUI, enable the cursor (#15363)
d056f22 
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
@DHowett
Add a fun new preview text in the SUI, enable the cursor (#15363)
edb55d7 
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 zadjii-msft mentioned this pull request Sep 18, 2023
Merged
zadjii-msft added a commit that referenced this pull request Sep 19, 2023
@zadjii-msft
Fix some smaller broadcast bugs (#15993)
f494d68 
## 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

Reviewers

@lhecker lhecker lhecker approved these changes

@PankajBhojwani PankajBhojwani PankajBhojwani approved these changes

Assignees

No one assigned

Labels

None yet

Projects

1.18 Servicing Pipeline
Status: Shipped

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@DHowett @lhecker @PankajBhojwani