microsoftgraph / microsoft-graph-toolkit

Authentication Providers and UI components for Microsoft Graph 🦒
https://docs.microsoft.com/graph/toolkit/overview
Other
964 stars 309 forks source link

mgt-peoplepicker in custom dialog enables hotkeys on listview in background [BUG] #2029

Closed anditee closed 10 months ago

anditee commented 1 year ago

Describe the bug If you use mgt-peoplepicker in an custom sp-dialog it enables the execution of hotkeys in the background of the dialog. For example you press ctrl + a while filling in a person you are searching for all items in the background get selected. Furthermore if you press the 's' key it opens up the share dialog from sharepoint online.

To Reproduce Steps to reproduce the behavior:

  1. Select Sharepoint Listitem
  2. Open custom dialog with listview custom command set
  3. Start typing in the mgt-peoplepicker and hitting keybinds like 'ctrl+a'
  4. Every listitem in the background of the dialog gets selected and not the filled in names

Expected behavior The focus stays within the dialog and doesn't execute the keybinds in background

Screenshots image image

Environment (please complete the following information):

Additional context

ghost commented 1 year ago

Hello anditee, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

sebastienlevert commented 1 year ago

This is an interesting bug! And it doesn't occur when the mgt-people-picker is not in use?

gavinbarron commented 1 year ago

does ctrl + a work with mgt-people-picker outside of a dialog?

anditee commented 1 year ago

@sebastienlevert it seems to be a combination of mgt-people-picker and the custom dialog. The dialog it self also does not disable this hotkeys. But only with people-picker (not any other type of input) you can execute those commands while writing. In my opinion the focus of the mgt-people-picker does not stay within the input. For example if you try to write test on the s it opens up the share dialog from the listview item in the background. So we are not able to type 's' or select the text in the input with ctrl + a without having the bug shown in the screenshots.

anditee commented 1 year ago

@gavinbarron within the microsoft graph toolkit playground ctrl + a works like expected and selects previous typed in text and you can hit backspace and delete the text you entered.

image image

PaoloPia commented 1 year ago

Hi @gavinbarron and @sebastienlevert, I confirm the described behavior/bug. I have an SPFx custom solution where we are using @microsoft/mgt-spfx@2.11.2 and @microsoft/mgt-react@2.11.2 in a dialog of SPFx dialog framework. When searching for people in the PeoplePicker control from a dialog triggered by an SPFx UI Custom Action, if users press "i" the info panel of the selected items pops up on the right side of the screen and the "i" char gets lost from the search query, if users press "s" the share item dialog pops up and the "s" character does not affect the search query, if users press ctrl+a all the items get selected. Smells like a bug. Thanks.

sebastienlevert commented 1 year ago

@PaoloPia have you tried with MGT 3.x? I'm curious to see if the issue is still present or if it's a weird behavior coming from the SharePoint page. Thanks!

PaoloPia commented 1 year ago

Hi @sebastienlevert, I've just created a very simple repro of the issue and unfortunately it occurs with 3.x, too. Here you can find it: https://github.com/PaoloPia/mgt-hotkeys-issue. In case you have time to play with it ...

Notice that I'm not using component disambiguation in the above sample, because I'm relying on the SPFx Dialog Framework to host the MGT components and not a "classic" Web Part + React component. IMHO, it should be worth providing a sample about how to do component disambiguation with dialogs, too.

Thanks mate!

sebastienlevert commented 1 year ago

Thanks @PaoloPia, really appreciate it! Adding @gavinbarron for disambiguation on dialogs!

PaoloPia commented 1 year ago

Hi folks, I know that this has been and still is a very busy timeframe. However, I'm just wondering if there is any update on this topic and in particular any ETA for a potential fix. Unfortunately, we have a tight schedule for a project that is impacted by this bug. Thanks!

sebastienlevert commented 1 year ago

We had discussions with the internal ODSP team on this and while there are acknowledgements on the issue (using shadow roots / web components in SPFx solutions), there is no ETA on a fix. Can I ask you to create an issue on the sp-dev-docs repo also referencing this issue? It's something that will need to be fixed on the accessibility side of ODSP.

Until then, I can't find a reasonable temporary fix unless you want to make some real wizardry in the eventing system.

sebastienlevert commented 10 months ago

There is an open issue here: https://github.com/SharePoint/sp-dev-docs/issues/9005. No work is required on the MGT side and I'll be closing this issue for now. Feel free to open if you think we should reconsider. Thanks!