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

fix: to #182266, Corrected Webview hasFocus check #182279

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

Conversation

yiliang114
Copy link
Contributor

Close #182266.

@mjbvz mjbvz added this to the June 2023 milestone May 15, 2023
if (viewContainerId && viewContainerId === activePanel?.getId()) {
return true;
}

const container = this.getContainer(part);

return !!container && isAncestorUsingFlowTo(activeElement, container);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Have you investigated why isAncestorUsingFlowTo doesn't work in this case? Ideally this part of the code shouldn't have to know anything webview specific

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have you investigated why isAncestorUsingFlowTo doesn't work in this case? Ideally this part of the code shouldn't have to know anything webview specific

Yes, I commented on #182266 (comment) earlier.

it looks like the activeElement is incorrect when Webview clicked

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have you investigated why isAncestorUsingFlowTo doesn't work in this case? Ideally this part of the code shouldn't have to know anything webview specific

Have you investigated why isAncestorUsingFlowTo doesn't work in this case? Ideally this part of the code shouldn't have to know anything webview specific

I think what you said is right. Ideally, there should be no special treatment for it. I'll try to see if activeElement value can be corrected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have you investigated why isAncestorUsingFlowTo doesn't work in this case? Ideally this part of the code shouldn't have to know anything webview specific

hi @mjbvz activeElement looks like a read-only property, we don't seem to be able to actively modify the value for it.

@mjbvz mjbvz modified the milestones: June 2023, July 2023 Jun 26, 2023
@mjbvz mjbvz modified the milestones: July 2023, August 2023 Jul 24, 2023
@yiliang114 yiliang114 requested a review from mjbvz August 15, 2023 16:22
@sbatten sbatten modified the milestones: August 2023, September 2023 Aug 29, 2023
@sbatten sbatten modified the milestones: September 2023, October 2023 Sep 25, 2023
@sbatten sbatten modified the milestones: October 2023, November 2023 Oct 25, 2023
@yiliang114 yiliang114 force-pushed the fix/webview-view-container-toggle branch from ec38a3d to c30cebe Compare November 28, 2023 11:08
@mjbvz mjbvz modified the milestones: November 2023, December 2023 Nov 29, 2023
@mjbvz mjbvz modified the milestones: December / January 2024, February 2024 Jan 23, 2024
@sbatten sbatten removed their assignment Feb 20, 2024
@mjbvz mjbvz modified the milestones: February 2024, On Deck Feb 21, 2024
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.

The Toggle Command of Webview can only open, but not close panel
3 participants