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

remove legacyIndicatorMenu #175933

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

remove legacyIndicatorMenu #175933

wants to merge 3 commits into from

Conversation

aeschli
Copy link
Contributor

@aeschli aeschli commented Mar 2, 2023

Fixes #175930

@aeschli aeschli self-assigned this Mar 2, 2023
@aeschli aeschli enabled auto-merge (squash) March 2, 2023 17:33
@vscodenpa vscodenpa added this to the March 2023 milestone Mar 2, 2023
joyceerhl
joyceerhl requested changes Mar 2, 2023
View reviewed changes
Copy link
Collaborator

@joyceerhl joyceerhl left a comment

Choose a reason for hiding this comment

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

vscode.dev still uses this and there are a few other consumers in the wild.

Tyriar
Tyriar previously approved these changes Mar 2, 2023
View reviewed changes
@Tyriar Tyriar disabled auto-merge March 2, 2023 17:59
@aeschli aeschli enabled auto-merge (squash) March 3, 2023 11:01
@aeschli aeschli requested a review from joyceerhl March 6, 2023 16:44
@aeschli
Copy link
Contributor Author

aeschli commented Mar 10, 2023

@joyceerhl Please have a look. I now removed StatusBarWindowIndicatorMenu and replaced its usages to StatusBarRemoteIndicatorMenu
In the web API, we still use the term window indicator even if we call it RemoteIndicator in our code. IMO that's ok, and I don't want start a swirl. There are two more web APIs also use that term.
For the download command to use StatusBarRemoteIndicatorMenu I added a new group general. These commands will be grouped at the end of the remote menu.

@aeschli aeschli modified the milestones: March 2023, April 2023 Mar 20, 2023
joyceerhl
joyceerhl reviewed Apr 21, 2023
View reviewed changes
Copy link
Collaborator

@joyceerhl joyceerhl left a comment

Choose a reason for hiding this comment

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

I think the commands from vscode.dev need to be updated to have a group, otherwise they'll get silently dropped from the remote menu with this change.

@aeschli aeschli modified the milestones: April 2023, Backlog Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joyceerhl joyceerhl joyceerhl requested changes

@Tyriar Tyriar Tyriar left review comments

Requested changes must be addressed to merge this pull request.

Assignees

@aeschli aeschli

Labels
None yet
Projects
None yet
Milestone
Backlog
Development

Successfully merging this pull request may close these issues.

Clean up legacy remote indicator menu
4 participants
@aeschli @Tyriar @joyceerhl @vscodenpa