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

Combine pane taskbar states in the taskbar #10090

Closed
Tracked by #6700
zadjii-msft opened this issue May 13, 2021 · 4 comments · Fixed by #10755
Closed
Tracked by #6700

Combine pane taskbar states in the taskbar #10090

zadjii-msft opened this issue May 13, 2021 · 4 comments · Fixed by #10755
Assignees
@zadjii-msft
Labels
Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) 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.11

Comments

@zadjii-msft
Copy link
Member

It doesn't seem like a bad idea to combine these states somehow (with a setting), but we'd want to make sure we're clear what happens in cases like:

  • one tab has progress=N%, another tab has no progress
  • one tab has progress=N%, another tab has progress=M%
  • one tab has progress=N%, another tab has progress=indeterminate
  • one tab has progress=N%, another tab has progress=M% AND is in the error state

etc.

Sure, we have to specify this exhaustively.

It is specified in Taskbar API here how progress bars are combined for multiple windows collapsed into single button. For consistency it would be good idea to duplicate the behavior.

Originally posted by @tringi in #6700 (comment)
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels May 13, 2021
@zadjii-msft zadjii-msft added Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. labels May 13, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label May 13, 2021
@zadjii-msft zadjii-msft added this to the Terminal v2.0 milestone May 13, 2021
@zadjii-msft zadjii-msft mentioned this issue May 13, 2021
Open
4 tasks
@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label May 13, 2021
@zadjii-msft zadjii-msft self-assigned this Jul 13, 2021
@zadjii-msft
Copy link
Member Author

zadjii-msft commented Jul 14, 2021 •
edited
Loading

Interesting. There are already some other weird edge cases about the progress state I didn't notice before, but are making testing.... tricky.

first

  • set one pane to indeterminate, the other to warning,75%. If you focus the warning pane, the taskbar will show warning,75%. If you then focus the indeterminate pane, the taskbar will switch to warning,100%. Focus another tab, then back to the tab with the indeterminate pane - correctly shows indeterminate in the TB.
  • same will repro with error,x%.
  • That will not repro with normal,y%. Interesting.

These were fixed in d5a14f9.

second

  • Setting the progress in a pane doesn't always update the tab/window? weird though I can't get this one to repro anymore.

more to follow...

@zadjii-msft
Copy link
Member Author

I'm not sure I totally love this, the way it's exactly specified by the taskbar API. For them, they always combine the states of an entire group of windows, with a priority order. So if there's multiple windows with different states, then "Error" is more important than "Warning", more than "Normal", more than "Indeterminate".

IMO, I'd almost expect more nuance to this. It feels weird to have the focused pane be set to indeterminate, then an adjacent pane set to (normal,25), and have the tab and taskbar show (normal,25).

image

I kinda think that the focused pane should always take priority, if it has progress set at all. If it doesn't, then I think we should fall back to the "combine" behavior. Same with the window itself - if the focused tab has a set progress state, let's use that one. Otherwise, we'll fall back to combining the states of all the other tabs.

Current progress is in dev/migrie/b/progress-bugs

@zadjii-msft zadjii-msft added the Needs-Discussion Something that requires a team discussion before we can proceed label Jul 14, 2021
@tringi
Copy link

tringi commented Jul 14, 2021 •
edited
Loading

The Taskbar rules, prioritizing errors, are designed to bring attention to issues the user can solve, or should be notified.

For example: It would be irritating if you had put two copy operations on the background, and once one had finished, the button would turn from green to orange/red, to indicate prompt (e.g. overwrite) at 1%, which you could've solved much sooner and saved time.

@zadjii-msft zadjii-msft removed the Needs-Discussion Something that requires a team discussion before we can proceed label Jul 19, 2021
@zadjii-msft zadjii-msft mentioned this issue Jul 22, 2021
Merged
3 tasks
@ghost ghost added the In-PR This issue has a related PR label Jul 22, 2021
@ghost ghost closed this as completed in #10755 Aug 10, 2021
@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 Aug 10, 2021
ghost pushed a commit that referenced this issue Aug 10, 2021
@zadjii-msft
Combine progress states in the tab, taskbar (#10755)
a14b6f8 
## Summary of the Pull Request
![background-progress-000](https://user-images.githubusercontent.com/18356694/126653006-3ad2fdae-67ae-4cdb-aa46-25d09217e365.gif)

This PR causes the Terminal to combine taskbar states at the tab and window level, according to the [MSDN docs for `SetProgressState`](https://docs.microsoft.com/en-us/windows/win32/api/shobjidl_core/nf-shobjidl_core-itaskbarlist3-setprogressstate#how-the-taskbar-button-chooses-the-progress-indicator-for-a-group). 

This allows the Terminal's taskbar icon to continue showing progress information, even if you're in a pane/tab that _doesn't_ have progress state. This is helpful for cases where the user may be running a build in one tab, and working on something else in another.

## References

* [`SetProgressState`](https://docs.microsoft.com/en-us/windows/win32/api/shobjidl_core/nf-shobjidl_core-itaskbarlist3-setprogressstate#how-the-taskbar-button-chooses-the-progress-indicator-for-a-group)
* Progress mega: #6700 

## PR Checklist
* [x] Closes #10090
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated

## Detailed Description of the Pull Request / Additional comments

This also fixes a related bug where transitioning from the "error" or "warning" state directly to the "indeterminate" state would cause the taskbar icon to get stuck in a bad state.

## Validation Steps Performed

<details>
<summary><code>progress.cmd</code></summary>

```cmd
@echo off
setlocal enabledelayedexpansion

set _type=3
if (%1) == () (
    set _type=3
) else (
    set _type=%1
)



if (%_type%) == (0) (
    <NUL set /p =�]9;4�
    echo Cleared progress
)
if (%_type%) == (1) (
    <NUL set /p =�]9;4;1;25�
    echo Started progress (normal, 25^)
)
if (%_type%) == (2) (
    <NUL set /p =�]9;4;2;50�
    echo Started progress (error, 50^)
)
if (%_type%) == (3) (
    @Rem start indeterminate progress in the taskbar
    @Rem this `<NUL set /p =` magic will output the text _without a newline_

    <NUL set /p =�]9;4;3�
    echo Started progress (indeterminate, {omitted})
)
if (%_type%) == (4) (
    <NUL set /p =�]9;4;4;75�
    echo Started progress (warning, 75^)
)

```

</details>
@ghost
Copy link

ghost commented Aug 31, 2021

🎉This issue was addressed in #10755, which has now been successfully released as Windows Terminal Preview v1.11.2421.0.: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

@zadjii-msft zadjii-msft

Labels
Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) 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.11
Development

Successfully merging a pull request may close this issue.

Combine progress states in the tab, taskbar microsoft/terminal
3 participants
@DHowett @zadjii-msft @tringi