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

Editor context menu badly positioned #122035

Closed
isidorn opened this issue Apr 23, 2021 · 8 comments
Closed

Editor context menu badly positioned #122035

isidorn opened this issue Apr 23, 2021 · 8 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders ios-ipados verified Verification succeeded
Milestone

Comments

@isidorn
Copy link
Contributor

isidorn commented Apr 23, 2021

iPad, sometime I can get the editor to render the context menu is a wrong place -> top left corner.
Easy to reproduce: attach an external mouse and use the right click to trigger a menu in the editor

@rebornix might you have an idea what this is? You can also give me a code pointer so I investigate futher..

851E6B1C-DA31-4FD5-A24C-D5BA121AAC6F

@isidorn isidorn added bug Issue identified by VS Code Team member as probable bug ios-ipados labels Apr 23, 2021
@isidorn isidorn added this to the May 2021 milestone May 6, 2021
@isidorn
Copy link
Contributor Author

isidorn commented May 7, 2021

@rebornix I forgot where we settled on this one. Do you plan to investigate, or should I?
Let me know what works for you. Thanks!

@rebornix
Copy link
Member

rebornix commented May 7, 2021

@isidorn I can investigate

@rebornix rebornix added the debt Code quality issues label May 7, 2021
@isidorn
Copy link
Contributor Author

isidorn commented May 26, 2021

I have investigated into this and my finding is the following:

  1. The context menu gets shown properly, however a keyboard wants to pop up from the bottom, causing a resize, causing a relayout of the context menu
  2. The relayout of the context menu sets both X and Y coordinates to zero here
  3. The reason for this seems to be that the Context Menu is living in the shadow dom, and thus when the relayout happens the coordinates are not properly fetched
  4. If I change to not use the shadowDom here the bug is fixed

@sbatten what is the benefit of using the shadowDom. Is it all right if we just do not use the shadow dom for the editor context menu on the iPad

@sbatten
Copy link
Member

sbatten commented May 26, 2021

shadowDom preserves focus most closely with native dialogs by rendering the context menu as a child of the editor. trying to remember, but I think this is necessary for notebooks because focus in each cell is important. I do not recall clearly for this case.

@isidorn
Copy link
Contributor Author

isidorn commented May 28, 2021

Okey. So I change that on iOS we do not use the shadowDom for the editor. Since without that the context menu positioning is broken on iOS.

@isidorn isidorn removed the debt Code quality issues label May 28, 2021
@rebornix
Copy link
Member

rebornix commented Jun 1, 2021

@isidorn @sbatten thanks for investigating into this, I didn't finish this in time. Having the context menu rendered in the shadow DOM avoid unnecessary blur and this pattern is used in multiple places, I wonder if we would run into similar issues in the future if we don't have it fixed properly in context menu. Also on iOS, right click in a notebook cell editor causes the notebook to lose focus, a left click followed by right click doesn't bring the focus back to the notebook/cell.

For this particular issue, the position of the context menu is correct.

@isidorn
Copy link
Contributor Author

isidorn commented Jun 1, 2021

An alternative workaround fix is to not react on relayouts when we get x and y to be 0.
Though I am open for suggestions.

@isidorn
Copy link
Contributor Author

isidorn commented Jun 4, 2021

To verify:

  1. Open VS Code on the iPad
  2. Have focus in the explorer
  3. Right click in the editor -> make sure the context menu is nicely positioned

@rebornix rebornix added the verified Verification succeeded label Jun 7, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jul 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders ios-ipados verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

4 participants
@rebornix @isidorn @sbatten and others