microsoftgraph / microsoft-graph-toolkit

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

[BUG] LocalizationHelper with disambiguation #3196

Open patrikhellgren opened 3 months ago

patrikhellgren commented 3 months ago

Describe the bug When using MGT 4.2.1 in a SPFx solution (React) using disambiguation (which is working), LocalizationHelper is not working the way the documentation says. I spent a few hours troubleshooting why LocalizationHelper wasn't working to set my own localized strings for the people-picker component until I had a look at the source code and figured out that I had to specify the disambiguated component name in the LocalizationHelper strings object. I would have expected that LocalizationHelper followed the disambiguation.

To Reproduce Steps to reproduce the behavior:

  1. Create a SPFx solution using React
  2. Add MGT to the solution
  3. Configure disambiguation for MGT according to the documentation
  4. Use PeoplePicker component in the solution
  5. Add LocalizationHelper and configure custom localization for the "people-picker" component like in the documentation
  6. The LocalizationHelper strings object will not override the ones in PeoplePicker
  7. Change the custom localization to be for string-used-for-disambiguation-people-picker instead
  8. Now the strings in PeoplePicker will be overridden

Expected behavior I expected LocalizationHelper to account for disambiguation. If this is by design then the documentation should clearly mention the intended behavior when using disambiguation, both under Localization and under Disambiguation.

Screenshots

Environment (please complete the following information):

Additional context

musale commented 3 months ago

Oh @patrikhellgren thank you for bringing this to our attention, ~embarrassingly~ again. The documentation for mgt-spfx needs an update to reflect this. We have this in our backlog now.

A follow-up question for you, you fixed this by changing the string to string-used-for-disambiguation-people-picker and it worked or what does step 7 in your repro mean? An initial issue https://github.com/microsoftgraph/microsoft-graph-toolkit/issues/3151 had a different approach. I would like to get an idea of a good way to have this working for spfx before updating the docs.

patrikhellgren commented 3 months ago

@musale I noticed the issue #3151 during my troubleshooting but upgrading to 4.2.1 did not resolve my issue. Also disambiguation was never mentioned in that issue so I'm guessing that he used the components without disambiguation. I was probably a bit unclear in my repro steps so I'll try to explain some more.

The documentation says to use this format for the localization object:

"people-picker": {
  inputPlaceholderText: "Start typing a name",
  noResultsFound: `We didn't find any matches.`,
  loadingMessage: "Loading..."
}

I never got that working and I also tried setting the strings on a global level like the following but that was not working either:

LocalizationHelper.strings = {
  noResultsFound: "We didn't find any matches"
}

I finally figured out that a localization object looking like the following worked ok. In this case the disambiguation string would have been set to "string-used-for-disambiguation":

"string-used-for-disambiguation-people-picker": {
  inputPlaceholderText: "Start typing a name",
  noResultsFound: `We didn't find any matches.`,
  loadingMessage: "Loading..."
}

I also would say that this has nothing to do with SPFx specifically since the components was registered ok with the disambiguated name and they were correctly displayed by the code, just not localized. You can see how the working syntax is used here from line 400 https://github.com/microsoft-search/pnp-modern-search/blob/develop/search-parts/src/webparts/searchVerticals/SearchVerticalsWebPart.ts.

Like this:

LocalizationHelper.strings = {
  _components: {
    "pnp-modern-search-people-picker": {
      inputPlaceholderText: webPartStrings.PropertyPane.Verticals.AudienceInputPlaceholderText,
      noResultsFound: webPartStrings.PropertyPane.Verticals.AudienceNoResultsFound,
      loading: webPartStrings.PropertyPane.Verticals.AudienceLoading
    }
  }
};

In the same class the components are registered by a call to this helper function:

const DISAMBIGUATION = "pnp-modern-search";

export const loadMsGraphToolkit = async (context: WebPartContext) => {
    // Load Microsoft Graph Toolkit dynamically
    const { customElementHelper } = await import(
      /* webpackChunkName: 'microsoft-graph-toolkit' */
      '@microsoft/mgt-element/dist/es6/components/customElementHelper'
    );

    customElementHelper.withDisambiguation(DISAMBIGUATION);

    const component = window.customElements.get(`${customElementHelper.prefix}-person`);
    if (!component) {
        const { Providers } = await import(
          /* webpackChunkName: 'microsoft-graph-toolkit' */
          '@microsoft/mgt-element/dist/es6/providers/Providers'
        );

        const { registerMgtComponents } = await import(
          /* webpackChunkName: 'microsoft-graph-toolkit' */
          '@microsoft/mgt-components/dist/es6/registerMgtComponents'
        );

        if (!Providers.globalProvider) {
            const { SharePointProvider } = await import(
                /* webpackChunkName: 'microsoft-graph-toolkit' */
                '@microsoft/mgt-sharepoint-provider/dist/es6'
            );

            Providers.globalProvider = new SharePointProvider(context);
        }
        registerMgtComponents();
    }
}

Then also In the same file the PeoplePicker component is lazy-loaded using this:

const PeoplePicker = React.lazy(() => 
    import(/* webpackChunkName: 'microsoft-graph-toolkit' */ '@microsoft/mgt-react/dist/es6/generated/people-picker')
        .then(({ PeoplePicker }) => ({ default: PeoplePicker }))
);

And then the component is rendered using this code:

return (
  React.createElement("div", null,
    React.createElement(React.Suspense, { fallback: React.createElement("div", null, webPartStrings.PropertyPane.Verticals.AudienceLoading) },
      React.createElement(PeoplePicker, {
        defaultSelectedGroupIds: item.audience || [],
        selectionMode: "multiple",
        type: "group",
        selectionChanged: onSelectionChanged,
      })
    )
  )
)

After this everything seems to working ok. Hopefully the example above is enough for you to improve the documentation. The current MGT documentation for SPFx seems to be only for pretty simple webpart usage, when used in more complex ones (like PnP Modern Search) it's a bit trickier but maybe this gives some insights on how it could be used. In that project MGT is used both as custom web components used in Handlebars templates and as React components like in my example above.

musale commented 3 months ago

@patrikhellgren thank you for the detailed explanation. This has shed more light on elements of disambiguation we had not documented. We shall update this accordingly and get out improved documentation on this area.