Skip to content

Conversation

@aqibshabir
Copy link
Contributor

This PR fixes a CSS z-index conflict in mobile view where the aside is overlapping the sidebar.

  • The aside had a z-index of 1000, which caused it to appear above the sidebar (z-index: 99).
  • Changed the aside z-index to 98, ensuring it stays behind the sidebar in mobile view.

@aqibshabir
Copy link
Contributor Author

@microsoft-github-policy-service agree

@jakebailey jakebailey added the deploy-preview Enables automatic deployments to preview environments on a PR label Dec 2, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Dec 2, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://victorious-plant-05c166c10-3282.centralus.5.azurestaticapps.net

@jakebailey
Copy link
Member

This does seem like an improvement but https://victorious-plant-05c166c10-3282.centralus.5.azurestaticapps.net/docs/handbook/declaration-files/templates/module-class-d-ts.html shows that the sidebar still overlaps content 😞

@aqibshabir
Copy link
Contributor Author

This does seem like an improvement but https://victorious-plant-05c166c10-3282.centralus.5.azurestaticapps.net/docs/handbook/declaration-files/templates/module-class-d-ts.html shows that the sidebar still overlaps content 😞

The solution I provided (almost 2 months ago) deals with the z-index conflict with the content appearing within the sidebar when the sidebar is active.

Having taken a look at the link provided, I'm slightly puzzled. You mentioned that the sidebar overlaps the content - isn't that the desired outcome when you select the hamburger menu - for the sidebar to be on top of the content?

Could you clarify what the expected behavior should be? I want to ensure we’re aligned and address any remaining concerns effectively. I appreciate your follow-up on this.

@jakebailey
Copy link
Member

Screenshot_20250126-023043

What I see is this on loading that page

@jakebailey
Copy link
Member

Now, this issue is certainly unrelated to your change since I'm pretty sure this is a regression introduced in some other change, but it is a little unfortunate to fix the z-index and then it still covers content 😄

@jakebailey
Copy link
Member

This PR affects code added in #3236; I don't see it breaking the accessibility fix in my test, so that's good.

@jakebailey jakebailey merged commit c147852 into microsoft:v2 Jan 26, 2025
8 checks passed
@aqibshabir
Copy link
Contributor Author

Now, this issue is certainly unrelated to your change since I'm pretty sure this is a regression introduced in some other change, but it is a little unfortunate to fix the z-index and then it still covers content 😄

Thank you for pointing that out! I did notice the issue, and I agree—it’s not ideal to leave it as is. To address this, I’ll work on making the site more mobile-friendly and will submit a pull request in the coming days with a proposed improvement. Looking forward to your feedback once it’s ready! 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deploy-preview Enables automatic deployments to preview environments on a PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants