Skip to content

Conversation

@a-stewart
Copy link
Contributor

This PR fixes #143197

Logic based on if the window is standalone should be based on if it is currently standalone, rather than if it was standalone when VS Code was started.

This change also moves addMatchMediaChangeListener from browser.ts to dom.ts so that it can be used in dom.ts without creating a circular dependency.

import { withNullAsUndefined } from 'vs/base/common/types';
import { URI } from 'vs/base/common/uri';

export { addMatchMediaChangeListener } from 'vs/base/browser/browser';
Copy link
Member

Choose a reason for hiding this comment

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

Why is this being re-exported?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As some things import addMatchMediaChangeListener from dom.ts.

If you prefer I could update all the imports to point at browser.ts?

Copy link
Member

Choose a reason for hiding this comment

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

Yes why not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, sorry for the delay

@bpasero bpasero modified the milestones: February 2022, On Deck, March 2022 Feb 20, 2022
@bpasero bpasero self-requested a review February 25, 2022 11:17
Copy link
Member

@bpasero bpasero left a comment

Choose a reason for hiding this comment

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

LGTM. Arguably even though isStandalone is now dynamic, there are still a few places where we statically call this method on startup and then fail to reconcile the change. But I think your scenario should be covered.

@bpasero bpasero merged commit 12e6b0b into microsoft:main Feb 26, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

isStandalone is a const which is wrong if the state changes

2 participants