Reenable menu shadow for reporting menu

[sgiehl]

Description:

I've also included some css tweaks . Instead of a padding the pagewarp now only uses a margin. That way that space won't be included in UI tests of the pagewrap anymore. This will require an update of a lot screenshots (as the free space will be removed), but we will no longer have problems with the menu shadow on screenshots.

Review

• Functional review done
• Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
• Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
• Security review done
• Wording review done
• Code review done
• Tests were added if useful/possible
• Reviewed for breaking changes
• Developer changelog updated if needed
• Documentation added if needed
• Existing documentation updated if needed

[sgiehl]

The CSS tweaks I've added should help a lot around UI tests regarding the menu shadow being visible and also around the free space visible on some screens, when the menu is higher than the reports.
Note: Merging this would mean we need to update the sceenshots once in all plugins, but we would have less failures in plugins due to changes in the menu in the future.

@matomo-org/core-reviewers @AltamashShaikh @snake14 Would you mind taking a quick look at the changes and share your opinion?

[michalkleiner]

The changes in general look ok. As commented above, we can do flex: none on the lefthand menu and then we don't need to do any width calc on the pageWrap itself, it would always behave as needed.

I'm not a big fan of having values like 9px or 18px (not on 4, or even better, 8px grid), but that's a discussion for another time.

[michalkleiner]

Code change looks good. Some more UI test screenshots need updating.

[sgiehl]

I've now created PRs for the submodules where the UI tests were failing. Guess we should merge them and update the submodules in this PR before merging. The remaining failing UI tests are unrelated.

[michalkleiner]

This is good for me but would be good to get another 👀 from @matomo-org/core-reviewers

[sgiehl]

@AltamashShaikh @snake14 I'm going to merge this PR now. It will most likely cause a lot of failures in UI tests of plugins. All tests taking a screenshot of the pageWrap element will get (a bit) smaller. But the changes in this PR will also prevent that parts of the menu shadow will be visible on such screenshots (so any hacks around that will no longer be needed). Also the height of the pageWarp element isn't impacted by the menu height anymore. This should make those UI tests a bit more stable and more independent from changes in core.

[snake14]

@AltamashShaikh @snake14 I'm going to merge this PR now. It will most likely cause a lot of failures in UI tests of plugins. All tests taking a screenshot of the pageWrap element will get (a bit) smaller. But the changes in this PR will also prevent that parts of the menu shadow will be visible on such screenshots (so any hacks around that will no longer be needed). Also the height of the pageWarp element isn't impacted by the menu height anymore. This should make those UI tests a bit more stable and more independent from changes in core.

Thank you @sgiehl . That's great news. I've been hoping this would be merged soon. I'll start updating the plugins.

[snake14]

Oh. @sgiehl Are these changes going to be back-ported to 4.x? We currently have several plugins with failing builds due to the navigation shadow.

[sgiehl]

@snake14 We will most likely only backport security fixes to 4.x-dev. So this won't be backported.

[snake14]

@snake14 We will most likely only backport security fixes to 4.x-dev. So this won't be backported.

@sgiehl Just thought I'd check since the marketplace changes that broke the builds was backported. I'll simply update the 4.x screenshots if this fix isn't going to be applied to that branch.