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

Replace falsy tooltip with label #841

Merged
merged 1 commit into from
Jan 29, 2021
Merged

Conversation

bwateratmsft
Copy link
Contributor

@bwateratmsft bwateratmsft commented Jan 28, 2021

Needed to fix downstream microsoft/vscode-docker#2679. Alternatively, if microsoft/vscode#115337 is fixed, this will not be needed.

Because all AzExtTreeDataProvider now have resolveTreeItem defined, VSCode always attempts to resolve a tooltip. However, due to this code, if the resolved tooltip is falsy, then it will not replace 'Loading...' from a few lines above.

In the past the standard behavior for tooltips was that label would be used, if tooltip was falsy. So, in this code for resolving tooltip, we'll replace it with the label if it is falsy.

@bwateratmsft
Copy link
Contributor Author

bwateratmsft commented Jan 28, 2021

@nturinski @ejizba @StephenWeatherford Do you know if any extensions other than Docker have started using 0.38.4+ of vscode-azureextensionui? Any using that version will show 'Loading...' as the tooltip forever unless resolveTooltip comes back with something non-falsy, or unless the VSCode team fixes microsoft/vscode#115337. I assume 1.53.0 is due out relatively soon, so if we don't get quick traction from VSCode we should probably put this fix in place and update package users from 0.38.4 to 0.38.5.

@ejizba
Copy link
Contributor

ejizba commented Jan 28, 2021

Functions and App Service are on 0.38.4 I think, so we'll make sure to bump them. Thankfully Functions got bumped to 0.38.4 right after we released the extension last week

@StephenWeatherford
Copy link
Contributor

ARM is on an earlier version.

@bwateratmsft
Copy link
Contributor Author

If we don't hear back on that VSCode bug by say, mid next week, I'll go ahead and put this PR through. What do you think, @nturinski / @ejizba / @StephenWeatherford?

@bwateratmsft
Copy link
Contributor Author

bwateratmsft commented Jan 29, 2021

Per @fiveisprime the upstream fix will probably not be in the nearest upcoming VSCode release, so I'll go ahead and put this fix in.

When it is fixed in VSCode, I'd like to revert the change here, because what we'll be doing is essentially duplicating the logic to use label when tooltip is falsy, but I'd prefer to leave the decision of what to show with VSCode in that scenario. I'll create an issue to revert it later.

@bwateratmsft bwateratmsft merged commit 9c456f2 into main Jan 29, 2021
@bwateratmsft bwateratmsft deleted the bmw/replaceFalsyTooltip branch January 29, 2021 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants