SharePoint / sp-dev-docs

SharePoint & Viva Connections Developer Documentation
https://docs.microsoft.com/en-us/sharepoint/dev/
Creative Commons Attribution 4.0 International
1.24k stars 1.01k forks source link

SPFx ListViewCommandSet Extensions are no longer being loaded when switching list context in side navigation! #5704

Closed frevds closed 3 years ago

frevds commented 4 years ago

Category

Describe the bug

Okay, let't try that again in the right section.

I registered a ListViewCommandSet Extension for a list. But the extension is not being loaded, if I don't directly navigate to that list:

If I open the list page for a list for which the extension is activated, the extension gets loaded (constructor called, onInit called).

If I open the list page for another list for which the extension is not activated and then navigate to the list for which it is activated, using the navigation tree, the extension does NOT get loaded.

Also, if the extension was loaded for a list and the user switches to another list using the navigation, any extension activated for that list does not get initialized (constructor or onInit is not being called again).

Finally, the onListViewUpdated-Event never worked.

This issue makes list view command set extensions quite useless and breaks our product. Is there any fix coming for this or should we move on to generic extensions loaded for every page?

Steps to reproduce

Use the yo generator to create a new ListViewCommandSet and try on your own to get it loaded for a list if you don't directly navigate to it (in SharePoint Online), see details above.

Second problem: the onListViewUpdated-Event never fires, no matter what you do in the ListView (provided the extension was loaded at all).

Expected behavior

Extensions should be loaded for lists on which they are registered, when you navigate to the list (using the navigation). Currently they are only loaded if you directly enter the correct list when opening the browser page, not when navigating in the tree (where the page does not reload but merely React controls get rerendered). This behaviour did work before, but is now broken.

Second problem: Changes in the selection of the list view should fire the event. Maybe even changes to the list context (navigating).

Environment details (development & target environment)

Additional context

fonbrauzen commented 3 years ago

Well, workaround is nice, but it could be finally fixed? Actually now it is broken for both teams & communication sites.

westleyMS commented 3 years ago

This is being actively investigated for a fix. I will update once it is done.

fonbrauzen commented 3 years ago

the workaround, that allows to use list views in navigation with SPFX customisations and avoid this bug (place in navigation for list view): /sites/MySiteUrl/Lists/MyList/MyListView.aspx?viewid=MyViewGUID e. g. /sites/contoso/Lists/Test/MyView.aspx?viewid=0e59ef28-0c17-4aaa-b6d7-9221f0a40613

evlo commented 3 years ago

Still same behaviour on 1.11

/sites/siteName/Lists/ListName/ (full page load)
-vs-
/sites/siteName/Lists/ListName/AllItems.aspx (partial page load)

SharePoint/sp-dev-docs#5776 - this is basically same thing better explained This works NOTE: theory is Sharepoint does full page reload as Sharepoint needs to redirect to the view and not just use link in the navigation "as is"

"MyViewGUID" is not solution for us, as we do not know viewID

For our solution onInit is getting triggered on navigation, but context is from the previous page - page FROM where navigation occurred

I think workaround at least in some cases might be using window.location and parse it for the list url etc. But that might not work for everyone and is not great solution

fonbrauzen commented 3 years ago

@evlo "For our solution onInit is getting triggered on navigation, but context is from the previous page - page FROM where navigation occurred" had same issue, ended on tracking/manipulating window.history and make full page refreshes, this is of course +network and what's else, but at least SPFX is there

evlo commented 3 years ago

@fonbrauzen Thank you for the tip. How exactly are you doing full page refreshes? I only came up with window.location.reload() in onInit of the extension. Is there better way?

fonbrauzen commented 3 years ago

@evlo sorry, just normal refresh is enough, via window.location.href this triggers SPFX events, the only problem - unreliable events after nav link click, hence using of window.history events (unrelated to SPFX)

evlo commented 3 years ago

I have updated my previous post as some of the statements were found not to be true during testing it again - the solution with removing view (link to *.aspx) does work.

VesaJuvonen commented 3 years ago

We do apologize the massive delay on this. Right now fix should be available at 50% level across worldwide tenants and rest should be properly fixed on Wednesday, unless there are unexpected delays.

We had internal root cause analyses on this in previous days and are significantly increasing the investments on the validation, testing and reporting areas. Objective is to ensure that we get the lists/library issues with SPFx extensions properly fixed once and for all... and reduce the possibility of regressions on these areas.

Let's wait for confirmations on the issue being truly fixed, before we close this.

We do acknowledge that only a fraction of our customers and partners have premier support, but in general opening tickets using those channels will have much faster way to get things moving and fixed, given the more official and structural process. We are working for sure updates on the GitHub process as well.

If you see similar issues in future, please open a new issue, if this one has been already confirmed and close.

andrewconnell commented 3 years ago

A fix has been applied to a similar issue... see #6611... can you verify if this resolved your issue?

frevds commented 3 years ago

@andrewconnell @VesaJuvonen This looks very much improved, thank you. In my tests the onInit of the CommandSet was triggered now when navigating around, at least once per extended list. It also seems to work when coming from a clientside page (e.g. the homepage). I say seems to, because I think I saw a few glitches where it did not trigger, but I now fail to reproduce the issue (will keep trying, but so far it looks good). And also the issue of the two conflicting React frameworks seems to be gone (we had to reload the page when switching between a page with a clientside webpart on it and an extended list page with a command set on it using React - the error occured at the first useState but this seems to be no longer happening). The only real issue is, that if I have the same clientside webpart on two different pages, the webpart does NOT get reinitialized NOR does it get re-rendered, despite having different webpart properties set up. We have to have a timer running on the page to detect changes to the address and manually trigger a page reload. Would be great if this issue could be resolved (I think it should be related, but I can open a new ticket for it if necessary). Otherwise I'm confident to say we can close this ticket.

VesaJuvonen commented 3 years ago

Thank you @frevds for confirming. As moving between two pages issue on web part is a separate case, can you open a new issue on that for easier tracking. We try to avoid having two or more issues in one particular case as it's impossible to track them. Thanks.

westleyMS commented 3 years ago

@frevds are you aware of the navigated event? The web parts are not supposed to be re-loaded on a partial page navigation. They are supposed to register for the navigation event to re-asses what is displayed after a navigation occurs. Here is a simple logging sample- this.context.application.navigatedEvent.add(this, () => { console.log('Navigated Event:', window.location.href); });

frevds commented 3 years ago

@westleyMS No, I wasn't, thank you for the tip, however I cannot find that .application member in the WebPartContext of the SPFx version I'm using (1.10) and also not in the ListViewCommandSetContext (and this is not an ApplicationCustomizer). This would also not solve the problem with the webpart, unless the webpart's properties change as well (which they don't), so a self-triggered re-render would be able to use those defined on the other page. A navigation from page to page (not list to list) should be more consistent - unloading webparts and loading new webparts, no matter whether they are the same set. There might be other external dependencies and factors determining the content of the webpart. I have created a separate ticket for this issue: https://github.com/SharePoint/sp-dev-docs/issues/6786

waaromikniet commented 3 years ago

I have tested on one of my older tenants and there I still see the oninit() is fired only once when switching between lists in the quicklaunch.

Does this mean that this is by design? And I should use another route to display a item command button in my list?

westleyMS commented 3 years ago

@waaromikniet it is by design, once the command set loads for a list on a given page, it doesn't re-load, but does get the onListViewUpdated whenever a change is made on the list view such as when an item is selected. This enables you to run code when a change is made. This is only true of partial page navigation, and not when you do a refresh of the page, or if you do a full navigation.

ghost commented 3 years ago

Issues that have been closed & had no follow-up activity for at least 7 days are automatically locked. Please refer to our wiki for more details, including how to remediate this action if you feel this was done prematurely or in error: Issue List: Our approach to locked issues