Skip to content

Conversation

@CyrusNajmabadi
Copy link
Member

This feature was added back in 2022: #59779

We don't need the old UI anymore at this point.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner March 21, 2025 20:47
@ghost ghost added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Mar 21, 2025
@CyrusNajmabadi CyrusNajmabadi requested a review from ryzngard March 21, 2025 20:55
{
dashboard.FocusNextElement();
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

removed as it will always be false to get the RenameDashboard now, and that was the only codepath that did anything.

_trackedSession.ReplacementTextChanged -= InlineRenameSession_ReplacementTextChanged;
}

if (!_globalOptionService.GetOption(InlineRenameUIOptionsStorage.UseInlineAdornment))
Copy link
Member Author

Choose a reason for hiding this comment

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

always effectively true now, so this block does nothing.


internal abstract partial class AbstractRenameCommandHandler :
IChainedCommandHandler<TabKeyCommandArgs>,
IChainedCommandHandler<BackTabKeyCommandArgs>
Copy link
Member Author

Choose a reason for hiding this comment

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

tab handling was only handled by the Dashboard, not the Flyout, so this can all be removed.

// Disable new rename UI for now, it's causing these tests to fail.
// https://github.com/dotnet/roslyn/issues/63576
var globalOptions = await TestServices.Shell.GetComponentModelServiceAsync<IGlobalOptionService>(HangMitigatingCancellationToken);
globalOptions.SetGlobalOption(InlineRenameUIOptionsStorage.UseInlineAdornment, false);
Copy link
Member Author

Choose a reason for hiding this comment

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

this is concerning as it means our tests aren't validating proper behavior in user scenarios. if this fails, i will disable this test as we have a tracking issue on it already.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you disable the test please re-open the tracking issue

Copy link
Contributor

@ryzngard ryzngard left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for cleaning up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-IDE Needs UX Triage untriaged Issues and PRs which have not yet been triaged by a lead

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants