silverstripe / silverstripe-asset-admin

Silverstripe assets gallery for asset management
BSD 3-Clause "New" or "Revised" License
19 stars 78 forks source link

CMS 4 - Toast notifications are sporadically broken #1345

Closed emteknetnz closed 1 year ago

emteknetnz commented 1 year ago

.

Update: see comment below about entwine

Also affects CMS 5 for GraphQL5

Not broken when using GraphQL3

Shows up as failed behat tests in asset-admin e.g. tests/behat/features/manage-files.feature:104

Also a broken behat test in versioned-admin tests/Behat/features/list-view.feature:53

And a broken behat test in cms tests/behat/features/duplicate-a-page.feature:15

To replicate: Upload some files to the the file manager Bulk delete 1+ files

You should see a toast notification, usually you don't when using GraphQL 4/5 ... but not always sometimes they do show. Reloading the browser and repeating the exercise should mean the toast notifications don't show.

Marking as medium priority because it doesn't really break functionality as missing toast notifications don't prevent you from doing things

I've debugged to see where things got to using 2x dev environments running 4.13 + graphql 3 & graphql 4

Seems like something going wrong in the graphql/redux/apollo stack

g3 + g4
AssetAdmin:372
this.props.actions.toasts.success() is called

g3 + g4
ToastsActions.js:54
both dispatch action
ACTION_TYPES.DISPLAY
with payload
1 folders/files were successfully archived

g3 + g4
ToastReducer.js:85
both run appendToast()

g3
Toasts.js:16 was called
! g4
Toasts.js:16 was NOT called

PRs

emteknetnz commented 1 year ago

Update to this, appears to be related to entwine, specifically in vendor/silverstripe/admin/client/src/legacy/ToastsContainer.js

I worked this out by noticing in react dev tools that the ToastsContainer was sometimes missing

jQuery.entwine('ss', ($) => {
  $('body').entwine({
    onmatch() {
        const container = $('<div class="toasts-container"></div>');
        this.append(container);
        ReactDOM.render(<ToastsContainer />, container[0]);
    },
  });
});

Putting a console.log() in onmatch() and running yarn dev sporadically works (though mostly fails) in firefox and seems to always work in chromium on my local. It's probably sporadically failing in CI on google chrome.

It may or may not be related to this recent change https://github.com/silverstripe/silverstripe-admin/pull/1476

Commenting the "namespace" e.g. jQuery.entwine('ss', ($) => { seems this always works in firefox, and so does changing the namespace to something else e.g. 'ssb'

emteknetnz commented 1 year ago

Linked PR has been merged and tagged as 1.12.6