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

Initial implementation of a new 'lightweight' background-work-indicator that we can use in-situ in the editor. #59759

Merged

Conversation

CyrusNajmabadi
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi commented Feb 25, 2022

Followup to #59802

The model of this closely follows (including at the API level) the IUIThreadOperationContext model the editor already has for CommandExecutionContexts.

Namely, an operation (like goto-def, add-using-on-paste, etc.) can startup and create such a context object to represent the work they do. However, instead of that context object being backed by a threaded-wait-dialog (TWD) (modal, heavyweight, focus-stealing), they instead get one effectively backed by a rich tooltip (non-modal, lightweight, doesn't affect focus) in the code where they started the operation. The UI for thsi tooltip is still a work in progress. However, you can imagine it saying things like Navigating to definition (Esc to cancel) with the options for things like progress to be shown using a thin progress-spinner like we have in many other parts of the IDE.

Clients of hte IUIThreadOperationContext can treat it just like they would a TWD, adding scopes to it and setting the description/progress of sub-scopes. The tooltip will then batch up those changes and update the UI accordingly.

The indicator also tries to come with some batteries included. For example, it automatically hooks up to Esc being listened to to cancel the work it is doing. It can also auto-cancel on edits to the buffer (the default), or navigating away from taht doc (the default).

Looks like this:

image

@CyrusNajmabadi
Copy link
Member Author

Tagging @cdblake1

// controlling everything ourselves.
_toolTipPresenter = factory._toolTipPresenterFactory.Create(textView, new ToolTipParameters(
trackMouse: false,
ignoreBufferChange: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why turn this off, and handle cancelOnEdit ourselves? Is it just not working yet?

Copy link
Member Author

Choose a reason for hiding this comment

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

Primarily because when we handle it ourselves we go through our entire pipeline of steps (including cancel'ing the CTS for our operation). We don't have an effective way (that i could tell) to know that the tooltip was dismissed due to buffer change.

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought I remember seeing something on the spec, either an event, or a cancellation token that we could use (and created linked token from), but if not, then fair enough. I might have been looking at the V2 doc.

@CyrusNajmabadi CyrusNajmabadi changed the base branch from main to release/dev17.3 March 6, 2022 17:43
@sharwell
Copy link
Member

sharwell commented Mar 9, 2022

Integration tests showed a bunch of errors coming from this exception:

SetSite failed for package [IntegrationTestServicePackage]Source: 'Microsoft.VisualStudio.IntegrationTest.Setup' Description: Method not found: 'Microsoft.VisualStudio.Threading.JoinableTaskFactory Microsoft.VisualStudio.Shell.AsyncPackage.get_JoinableTaskFactory()'.
System.MissingMethodException: Method not found: 'Microsoft.VisualStudio.Threading.JoinableTaskFactory Microsoft.VisualStudio.Shell.AsyncPackage.get_JoinableTaskFactory()'.
   at Microsoft.VisualStudio.IntegrationTest.Setup.IntegrationTestServicePackage.<InitializeAsync>d__1.MoveNext()
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder.Start[TStateMachine](TStateMachine& stateMachine)
   at Microsoft.VisualStudio.IntegrationTest.Setup.IntegrationTestServicePackage.InitializeAsync(CancellationToken cancellationToken, IProgress`1 progress)
   at Microsoft.VisualStudio.Shell.AsyncPackage.<>c__DisplayClass20_0.<<Microsoft-VisualStudio-Shell-Interop-IAsyncLoadablePackageInitialize-Initialize>b__1>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.Threading.JoinableTask.<JoinAsync>d__76.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at Microsoft.VisualStudio.Services.VsTask.RethrowException(AggregateException e)
   at Microsoft.VisualStudio.Services.VsTask.InternalGetResult(Boolean ignoreUIThreadCheck)

I'm guessing this PR updated dependencies to versions not supported by CI?

@CyrusNajmabadi CyrusNajmabadi changed the base branch from release/dev17.3 to main-vs-deps March 9, 2022 16:31
@CyrusNajmabadi CyrusNajmabadi changed the base branch from main-vs-deps to release/dev17.3 March 9, 2022 16:31
@CyrusNajmabadi CyrusNajmabadi marked this pull request as ready for review March 15, 2022 15:31
@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner March 15, 2022 15:31
@CyrusNajmabadi CyrusNajmabadi merged commit fb71ede into dotnet:release/dev17.3 Mar 17, 2022
@CyrusNajmabadi CyrusNajmabadi deleted the backgroundWorkIndicator2 branch March 17, 2022 17:58
@sharwell sharwell added this to the 17.3 milestone Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants