Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Force mat-drawer to relative, otherwise service categories are not visible anymore. #92

Closed
wants to merge 1 commit into from

Conversation

asattler
Copy link

@asattler asattler commented Dec 7, 2023

In material ui .mat-drawer has 2 settings for position, first is "relative", followed by "absolute" which is overruling the relative. so mat-drawer has position: absolute, which leads to display issues (service categories not visible)

visual issue

MicrosoftTeams-image (1)

…sible anymore.

In material ui .mat-drawer has 2 settings for position, first is "relative", followed by "absolute" which is overruling the relative. so mat-drawer has position: absolute, which leads to display issues (service categories not visible)
@Mathnstein
Copy link
Collaborator

Good catch! Will implement internally. Bug Id 441227

@Mathnstein Mathnstein closed this Dec 7, 2023
@Mathnstein
Copy link
Collaborator

Hi @asattler - one of my developers has taken a look and seems to not be able to reproduce the bug in question and additionally does not see the solution from this PR changing much.

Could you provide a clearer repro of the problem + what browser you are using? (Looks like FF but just want to see if we can repro first)

08c1e491-2189-4bac-bc77-d19bcf9df813

@Mathnstein Mathnstein reopened this Dec 21, 2023
@asattler
Copy link
Author

Hey Cody,

I tried to check different ways for reproduction. I found out that in a clean copy of your v92-branch all looks good while my fork, with some customizations has the issue (reproducable in chrome + firefox).
So I created a new fork, re-inserted all my customizations and it still looks good.

My Fork was originally from v82 (v82 -> v90 -> v91 -> v92) and I remember having merge conflicts with going from v91 to v92 even in files I was I've never touched... I resolved all of them and it worked fine, so I didn't expect this to be an issue.
But from the current point of view I can only imagine that due resolving the merge conflicts (I already had a dicussion with Hanno about this, and we came to the conclusion that I should've made the merge differently) broke something.

So it seems that the ootb code of v92 is fine and something got messed up in my fork due merging....
Sorry for that!

@asattler asattler closed this Dec 22, 2023
@asattler asattler deleted the patch-1 branch January 3, 2024 12:58
@asattler
Copy link
Author

asattler commented Jan 3, 2024

@Mathnstein I was now able to find out what caused the issue. It was indeed related to customization, which I want to explain:
With ootb code I had issues with bigger custom forms, which got cut because they needed to much height.
This issue for the "cut up" is related to the fixed height on css-class ".page"
.page { display: flex; flex-direction: column; overflow: hidden; height: 100vh; }

So it seems like, to resolve my issue I've removed the fixed height and couldn't see any negative impact while doing this.
So with now knowing that it breaks the service category list, I would instead remove the overflow:hidden in css-class .pageContent - which also solves the issue. But, tbh, I am not happy that I need to do this..

From my point of view the design concept of the angular web portal, which (for me) seems to "avoid scrolling of the page, but scroll inside of containers" isn't working well. Companies still have 12" notebooks in place which might have really horrible resolutions (yes in the office there are external monitors). Just to give you an idea of "how bad it might look":
screenshot2

Hope you see my point, but at the end I at least have to say sorry again for not seeing that my original issue of this PR was related to a change of mine.

@Mathnstein
Copy link
Collaborator

Thanks for the extra info @asattler - I passed this off to my dev who worked on this + our UX team.

We're in the process of beautifying some of our pages and here was one that went through that process. But that is definitely an issue with smaller screen real estate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants