Bump nextcloud-dialogs from 3.1.2 to 3.1.4

[PVince81]

This brings in the "aria-live" attribute for accessibility.

To test:

1. Create a folder "abc"
2. Open a second browser window on the root
3. In the second window, rename "abc" to "def"
4. Go back to the first browser window and also rename "abc" to "def" to trigger an error
5. Inspect the notification

We can see that aria-live is there.

BUT there's also this weird button size now, perhaps due to my increased font sizes.
Not sure, but could be caused by nextcloud-libraries/nextcloud-dialogs#582 as it touches the button

[PVince81]

this is the state before this PR:

the close button did not have the border and also was properly aligned

[PVince81]

raised nextcloud-libraries/nextcloud-dialogs#631 to look into this.

let's not merge until this is solved

[PVince81]

for the record, this is how I updated the deps:

1. Edited package.json and set the version to 3.1.4
2. Ran: npm install @nextcloud/dialogs
3. Ran: make build-js-production
4. Pushed after testing

not sure why this would mess with icons.css and also produce those side effects

[PVince81]

/compile amend /

[nickvergessen]

git checkout master
npm ci
npm i @nextcloud/dialogs@^3.1.4
make build-js-production

All looks good here

[nickvergessen]

Also looks good now with what the bot commited

[PVince81]

I ran the same command like Joas locally but it produced a different result that leads to the same problem: shifted button + circle around the "x". I have node v14.17, so I'll investigate this separately.

Now I've retested with this PR with the bot's result, and the circle is gone.
The cross is still shifted though:

[PVince81]

we did more research and at first thought that this line was missing:

import '@nextcloud/dialogs/styles/toast.scss'

but adding it in the relevant places didn't fix the down-shifted cross icon

then we tried upgrading the notifications app: nextcloud/notifications#1235
and here we noticed that the cross icon was not shifted, but this was on the settings page.

now I noticed that even with the "notifications" app disabled, if I go to the settings page, it works fine with this on the console:

OC.Notification.show("is the cross icon shifted down ?")

running the above code in the console:

• on the settings page ✅
• in the files app: 🚫

so probably the files app has something else that is interfering with the notification

[PVince81]

with notifications app enabled with nextcloud/notifications#1235: works everywhere

with notifications app disabled: works on settings page, but broken on files app and dashboard

[PVince81]

DOM differences:

master:

the cross is a button and the parent has a flex on it

this PR:

no button, no flex

[PVince81]

this PR on the settings page:

same markup, but with an additional padding

[PVince81]

this PR with notifications app enabled, files app page:

[PVince81]

alright, I have compared the files app with and without the notifications app loadade:

1. files app without notifications app:

• toast.scss is loaded but it's an older version

2. files app with notifications app:

• toast.scss is loaded with the newer version as I can see this row https://github.com/nextcloud/nextcloud-dialogs/blob/master/styles/toast.scss#L83 present (as an example)

next up: check which toastify

[PVince81]

• without notifications enabled:

first is pointing to "webpack://./node_modules/@nextcloud/dialogs/styles/toast.scss"
second points to "https://vvortex.local/node_modules/@nextcloud/dialogs/styles/toast.scss"
BOTH contain the old version

• with notifications enabled:

the two first entries are loaded from "webpack://./node_modules/@nextcloud/dialogs/styles/toast.scss" which contains the correct version

the last one is loaded from "https://vvortex.local/node_modules/@nextcloud/dialogs/styles/toast.scss"
and only that one contains the old version

a bit weird....

[PVince81]

the local file in the workspace "./node_modules/@nextcloud/dialogs/styles/toast.scss" contains the wrong version despite the upgrade 🤔

[PVince81]

and the diff from this PR updates "core/css/server.css" and injects it with the correct version of CSS from toastify 🤯

[PVince81]

@nickvergessen our assumption that npm ci would clean up / upgrade node_modules correctly seems to be wrong.

on the local workspace I found that @nextcloud/dialogs/CHANGELOG.md was still on the old version.
deleting "node_modules" and re-running "npm ci" fixed that.

I wonder why we don't delete "node_modules" on make clean or at least make dev-setup ?

I'm a tiny bit worried that now having different version of the lib still seem to load the SCSS from all libs instead of leaving it aside and keeping only core/css/server.css

maybe @skjnldsv knows more

anyway, we now should move forward with the lib upgrade to reduce the number of a11y warnings

now recompiling...

[PVince81]

recompiling didn't make a difference...

there's still a sneaky thing serving an old version of nextcloud-dialogs despite the fact that the new one is present in "node_modules/@nextcloud/dialogs"

[PVince81]

% find . -iname \*"dialogs"
./apps-extra/spreed/.git/logs/refs/heads/bugfix/5192/force-leave-on-lock-warnings-dialogs
./apps-extra/spreed/node_modules/@nextcloud/dialogs
./apps-extra/viewer/node_modules/@nextcloud/dialogs
./apps-extra/guests/node_modules/@nextcloud/dialogs
./apps-extra/mail/node_modules/@nextcloud/dialogs
./apps-extra/contacts/node_modules/@nextcloud/dialogs
./apps-extra/groupfolders/node_modules/@nextcloud/dialogs
./apps-extra/files_downloadlimit/node_modules/@nextcloud/dialogs
./apps-extra/files_automatedtagging/node_modules/@nextcloud/dialogs
./apps-extra/recommendations/node_modules/@nextcloud/dialogs
./apps-extra/photos/node_modules/@nextcloud/dialogs
./apps-extra/calendar/node_modules/@nextcloud/dialogs
./apps-extra/user_migration/node_modules/@nextcloud/dialogs
./apps-extra/notifications/node_modules/@nextcloud/dialogs
./apps-extra/tasks/node_modules/@nextcloud/dialogs
./apps-extra/deck/node_modules/@nextcloud/dialogs
./apps-extra/text/node_modules/@nextcloud/dialogs
./node_modules/@nextcloud/dialogs

next up: disable all these apps

non-fun thing: after moving out "viewer" the cross icon disappears completely...

[PVince81]

I've tested locally with nextcloud/viewer#1301 (the lib update for the viewer app that one cannot disable) and after compiling that PR's assets, the cross icon is finally positioned properly 😮‍💨

[PVince81]

the viewer update went through and now everything is fine, so we can merge this PR!

[PVince81]

/compile amend /

[PVince81]

/compile amend /