search

dnnsoftware / Dnn.AdminExperience

DNN (formerly DotNetNuke) Combined Admin Experience
MIT License
17 stars 39 forks source link

PersonaBar closing action does not reset <body> overflow value properly #1030

Closed jan-jonas closed 5 years ago

jan-jonas commented 5 years ago

Description of bug

When the PersonaBar is open the CSS style "overflow: hidden" is applied on the tag. After closing the PersonBar the tag gets the styling "overflow: auto;" which is IMHO a problem as "auto" is not the default value for overflow (it's "visible"). In other words: After opening and closing the PB, the body tag isn't in the same state as it was before.

Steps to reproduce

  1. Inspect CSS overflow attribute on
  2. Open PersonaBar
  3. Close PersonaBar
  4. Inspect CSS overflow attribute on again (it's now "auto" and not the value it was in step 1)

Current result

Opening and closing the PersonaBar changes the CSS overflow attribute on

Expected result

After opening and closing the PersonBar, the CSS overflow attribute on is in the state it was before opening the PersonaBar.

Affected version

I only tested the following two versions, but most likely the problem exists in all versions that include the PersonaBar:

Affected browser

The Problem is not browser specific.

mitchelsellers commented 5 years ago

This is an interesting one, as determining any styles that applied to the tag can become a bit complicated. @valadas any ideas on a solution for this?

jan-jonas commented 5 years ago

IMHO, this is not about any style. It should be enough, if the PB remembers and restores the "overflow" state of the .

mitchelsellers commented 5 years ago

@jan-jonas Yes, it might be as simple as checking for explicit values on the <body> tag but I just want to be sure that there isn't a bigger issue/or possible issue

valadas commented 5 years ago

In my personal opinion, this modification of the body style should not be inlined. The persona bar adds a persona-bar-visible class but then modifies the inline style.

I think the persona bar should add persona-bar-visible when it is present and persona-bar-open when there is an open panel. Then we could move the styling into a stylesheet instead of inline such as:

body.persona-bar-visible: {margin-left: 80px;} body.persona-bar-open: {overflow:hidden;}

This offers better performance and more flexibility and avoids conflicts if other stuff also modifies the body (3rd party extensions?)

valadas commented 5 years ago

And it would have the added benefit that if we later decide to add a switch to show/hide the persona bar somehow, we just have to deal with the persona-bar-visible class to push or not push the site depending if it is visible or hidden. (This is something that I have implemented locally as a proof of concept but had issues with keyboard navigation, but I still plan on doing it right at some point, I think a lot of people would love such a feature.

mitchelsellers commented 5 years ago

I love that fix Daniel

jan-jonas commented 5 years ago

@valadas: Sounds good. These two classes persona-bar-visible and persona-bar-open are new, right? I don't see them in our Evoq version that bases on DNN 9.3.1. Having these classes would also allow the skin developers to consider the PersonBar status.

valadas commented 5 years ago

One already exists, the other would be new

jan-jonas commented 5 years ago

@valadas: Exists since when?

valadas commented 5 years ago

@jan-jonas I think since there was a placeholder to prevent the side moving during the persona bar transition into it's position somewhere in 9.2.x or 9.3.x :

image

valadas commented 5 years ago

Ok, so closing this one since the overflow part was done in #1164 and the classes idea are in another issue already.