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

[BUG] mgt-file-list always opens clicked item in new tab #2670

Closed
brugh opened this issue Aug 17, 2023 · 5 comments · Fixed by #2685
Closed

[BUG] mgt-file-list always opens clicked item in new tab #2670

brugh opened this issue Aug 17, 2023 · 5 comments · Fixed by #2685
Assignees
Labels
bug Something isn't working State: In Review

Comments

@brugh
Copy link

brugh commented Aug 17, 2023

Describe the bug
The default event itemClick always opens a new tab with the clicked item and is not preventable. in v2 i can subscribe to the event and create a new file-list-query that would reinitialize the mgt-file-list on the new path. that mechanism still works and the filelist opens on the new directory BUT it also opens a new tab with that directory in onedrive or sharepoint.

To Reproduce
add to index.html head:

  <script src="https://unpkg.com/@microsoft/mgt@3/dist/bundle/mgt-loader.js"></script>
  <script type="module" src="https://unpkg.com/@fluentui/web-components"></script>

and to the body:

  <mgt-msal2-provider client-id="xxx-xxx-xxx"></mgt-msal2-provider>
  <mgt-file-list ></mgt-file-list>
  <script>
    document.querySelector('mgt-file-list').addEventListener('itemClick', (e) => {
      e.preventDefault()
      console.log(e.detail)
    },);
  </script>

which logs the item details to console but opens a new tab regardless. adding a template to the file list should prevent the default event to fire according to the docs, but that makes no difference.

Expected behavior
only log the item to console (so we can do custom stuff with it) and NOT open a new tab all the time unless specifically requested.

Environment (please complete the following information):

  • OS: windows 11
  • Browser edge and chrome
  • Framework none
  • Context Office 365
  • Version v3
  • Provider Msal2Provider, no specific config
@brugh brugh added bug Something isn't working Needs: Triage 🔍 labels Aug 17, 2023
@github-project-automation github-project-automation bot moved this to Needs Triage 🔍 in Graph Toolkit Aug 17, 2023
@brugh
Copy link
Author

brugh commented Aug 17, 2023

addition; i added a 'debugger' as the first line of my event handler and the tab opens before it even gets to it so this event is non-preventable...

  document.querySelector('mgt-file-list').addEventListener('itemClick', (e) => {
    debugger;
    e.preventDefault();
    console.log(JSON.stringify(e.detail))
  });

@brugh
Copy link
Author

brugh commented Aug 17, 2023

just had a look at the source and for some reason the mgt-file-list tries to open the selected item if there's a webUrl, regardless of any setting or event. No idea why that would be default behavior without any option to opt out of that. A possible workaround would be to redefine the function that does that and handle opening only if the item is a file...

    document.querySelector('mgt-file-list').handleFileClick = () => {};

@gavinbarron
Copy link
Member

Thanks for letting us know the challenges you're facing @brugh

The file list is an area where we are working on plans for significant improvement, including providing native support for folder navigation.

Unfortunately, the current implementation does leave a bit to be desired.

Internally to a component there is no way of knowing if there is anything listening for a given event, it makes the loosely coupled event model both a blessing and a curse. The best we could do would be to add boolean attribute to the file list component, disable-open-on-click that suppresses the behavior you don't want (it was added due to a request to make the list more useful)

Would that work for your case?

@brugh
Copy link
Author

brugh commented Aug 18, 2023

yes, if I can set the open-on-click to false that would help. that would be the same behavior as v2 had for which i have a handler already that either changes the file-list-query to the ID of the selected item if it's a directory (and update my fluent-breadcrumb component), or open the item if it's a file (I'm adding an extra check that if the item is a office xml, i'll add it to the document where this app is an add-in for ;)). so i have that functionality already.
setting a boolean would be a cleaner solution than i have now by overwriting the internal handleFileClick. i'm assuming having an itemClick event and an internal file click handler we can rewrite would be serving the same purpose. i for one dont need both.

@gavinbarron
Copy link
Member

Great!

And to confirm, yes my thinking is that this boolean would only disable the opening behavior of the file item, the existing events would still be emitted as expected.

@gavinbarron gavinbarron moved this from Needs Triage 🔍 to Proposed 💡 in Graph Toolkit Aug 18, 2023
@gavinbarron gavinbarron moved this from Proposed 💡 to Todo 📃 in Graph Toolkit Aug 24, 2023
@gavinbarron gavinbarron self-assigned this Aug 24, 2023
@gavinbarron gavinbarron moved this from Todo 📃 to In Review 💭 in Graph Toolkit Aug 25, 2023
@github-project-automation github-project-automation bot moved this from In Review 💭 to Done ✔️ in Graph Toolkit Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working State: In Review
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants