pnp / mgt-samples

A curated collection of community-contributed samples using the Microsoft Graph Toolkit
https://pnp.github.io/mgt-samples/
MIT License
30 stars 13 forks source link

SPFX sample is confusing #12

Closed smolattack closed 10 months ago

smolattack commented 10 months ago

Sample

https://github.com/pnp/mgt-samples/tree/main/samples/app/sp-webpart

Author(s)

@gavinbarron @sebastienlevert

What happened?

The author for this sample does not seem to be listed in the ReadMe.md file

It is quite ironic that an article titled disambiguation has incomplete information and links to a very confusing sample.

https://github.com/pnp/mgt-samples/blob/fab3c392c1411ba4436948f4e64301deb8c4e104/samples/app/sp-webpart/src/webparts/mgtDemo/components/MgtDemo.tsx#L6C1-L7

  1. Confusion
  1. Grammar https://github.com/pnp/mgt-samples/blob/fab3c392c1411ba4436948f4e64301deb8c4e104/samples/app/sp-webpart/src/webparts/mgtDemo/components/MgtDemo.tsx#L9C1-L9C1

    If you wish to make used of element disambiguation you should:

We don't want to be spend all this time diagnosing issues about the wrong sample, do we?

Or maybe don't ask rhetorical questions in your instructions to begin with?

Steps to reproduce

1. 2. 3.

Expected behavior

Sample about disambiguation to have a single focus. How should someone using the latest version of an SPFx webpart import and use React graph toolkit components?

Developer environment

Windows

Browsers

Additional environment details

No response

sebastienlevert commented 10 months ago

Thanks for reporting the issue @smolattack! Some of these issues are valid and should be fixed. I'll send a PR that will fix some of them.

Here are a couple of comments:

  1. This is a miss when we moved to disambiguation. It absolutely should not refer to this but to the @microsoft/mgt-react package. This will be easier and fully take advantage of our disambiguation features.
  2. I'll be fixing those typos, thanks for reporting!