BUGFIX: Let unapplied changes overlay cover everything except inspector

[grebaldi]

fixes: #2670

The problem

The unapplied changes overlay is supposed to prevent all editing actions as long as there are pending changes in the inspector, so that the user is forced to make a decision of whether to apply or discard their changes and prevent them from accidentally losing their unsaved work.

Unfortunately, the unapplied changes overlay does not cover the entire user interface, but only the content canvas area:

This (unintentionally) allows the user to perform all kinds of actions (like changing the edit-/preview-mode, discarding changes, etc.) and potentially lose their progress.

The solution

The first part of the solution is the attempt to let the unapplied changes overlay cover the entire user interface (minus the inspector area):

This is done in three steps:

1. Move the overlay directly underneath the document body via ReactDOM.createPortal, so that it competes for z-index globally
2. Adjust the positioning of the &::after-element, so it covers the entire top bar
3. Set the --zIndex-Dialog variable to a value that allows only flash messages to be drawn on top of the overlay

Step 3 turned out to be a bit tricky, because of the logic manifested in packages/build-essentials/src/styles/styleConstants.js. I was unable to achieve the desired effect without changing the variables name from --zIndex-UnappliedChangesOverlay-Context to --zIndex-UnappliedChangesOverlay, which is possibly breaky to anyone who relies on the former (this isn't official API though and is very unlikely to have any noticable effect).

I also adjusted --zIndex-UnappliedChangesOverlay in cssVariables.css, expecting that this will be necessary for later versions.

The second part of the solution has to do with the unapplied changes dialog. After I've tested the overlay, I noticed that the semi-transparent dialog overlay also did not cover the entire user interface:

I had noticed this before and documented the faulty behavior in #2925. To make the unapplied changes mechanism work as intended, I decided to fix this z-index issue in the scope of this PR as well.

Similar to the unapplied changes overlay, I had to rename --zIndex-Dialog-Context to --zIndex-Dialog.

P.s.: The blurs you've undoubtedly noticed in the screenshots are being addressed over here: #3489

EDIT: E2E-tests revealed that I needed to adjust the z-index configuration for select box dropdowns as well. Hooray for E2E-tests! 😄

[mhsdesign]

I also adjusted --zIndex-UnappliedChangesOverlay in cssVariables.css, expecting that this will be necessary for later versions.

1000% wow thanks, Yes this is due to dropping the styleConstants.js in the future (8.3) #3366

and here is my description, as to why we have them also in the 7.3 branch ^^ #3438

[crydotsnake]

Tested and works as expected!

[mhsdesign]

Thanks ;) works as described (tested on 8.3 to make sure the new cssVariables work)

what do you think about making the overlay also cover the "select element" box as well as the publishing dropdown:

this would then also resolve: #3501

currently one can publish changes and also escape to the workspaces backend module via the selector.

another little detail i noticed on 8.3, is that the mouse becomes a pointer and the hover effect is triggered for the language select box

the language selectbox will also open, their items not be selectable:

probably an z-index issue

[grebaldi]

@mhsdesign:

what do you think about making the overlay also cover the "select element" box as well as the publishing dropdown:

Good point. I'd find that quite sensible (also think that <SelectedElement/> has actually no structural point in being part of the Inspector in the first place tbh.). It'd be also less risky compared to #3501.

If I get a couple of 👍 on this plan, I'll change it here and close #3501 :D

[mhsdesign]

Yes I believe it’s much easier to maintain than to have #3501 in our code base. Sorry that you went through the hassle of creating #3501 if we discard your work

[grebaldi]

Done :)

@mhsdesign:

Yes I believe it’s much easier to maintain than to have #3501 in our code base. Sorry that you went through the hassle of creating #3501 if we discard your work

No worries! I'm all for it: Less entropy, fewer troubles :)

[crydotsnake]

It works as expected!

I remembered one more tiny detail. If you follow the steps in the issue to reproduce this bug, even though the inspector is dimmed, it is still possible for me to publish my changes even though I still have changes in the inspector that I haven't saved.

Would it be a big effort to disable the publish button in such a case? Or should we discuss that maybe for Neos 9? Does it even make sense? 😅

[grebaldi]

@crydotsnake:

I remembered one more tiny detail. If you follow the steps in the issue to reproduce this bug, even though the inspector is dimmed, it is still possible for me to publish my changes even though I still have changes in the inspector that I haven't saved.

I can't reproduce that... In my setup, the publish button lights up orange, but if I click it, I get the "Unapplied Changes"-dialog as expected.

But then again I think it makes sense to give more visual indication here... I'll see what I can do :)

[mhsdesign]

@grebaldi thank you so much for your adjustments.

I think its fine to not show any additional visual indication in the publish button, and either way it could be done in a separate pr (if at all ^^)

I will try this out and see if it as expected overlays with the publish button and then id say this is ready to be merged!

@crydotsnake are you sure that you can still click the publish button? Could you please try to debug this if so, maybe by checking with other browsers and investigating the css styles in the dev tools.

[mhsdesign]

Bildschirmaufnahme.2023-06-12.um.20.51.29.mov

@crydotsnake it works on my mashine (well i cheated again as i tested it cherry-cheryy-lady picked on 8.3 :P

[mhsdesign]

Thanks it works now perfectly ;) And should survive the upmerge ^^

[crydotsnake]

Tested it again, and its working great!

[grebaldi]

I've just noticed a regression :(

The "Unapplied changes" overlay now covers the secondary inspector. This makes it impossible to crop images. I will come up with a fix soon.

[mhsdesign]

The "Unapplied changes" overlay now covers the secondary inspector. This makes it impossible to crop images. I will come up with a fix soon.

oh well good catch 😅 i was about to release it the next days - we def need e2e test for that??? Should have affected every secondary inspector right? - I forgot to test that as well ^^