left TOC z-index incorrect in wide viewport with toc.follow

[mhostetter]

Small issue, but I noticed the left TOC from python-apigen generated pages have a display issue. It appears the contents of the TOC appear on top of the page title "API Reference" (in my case) when the TOC is long enough.

Example

https://mhostetter.github.io/galois/latest/api/galois.ReedSolomon.k/

Scroll in the left TOC and observe the overlay on top of "API Reference".

[2bndy5]

At first glance, this seems related to the toc.sticky feature.

[2bndy5]

well, I was able to fix this in the dev console by adding the following css rule to the <label> element representing the text "API Reference":

z-index: var(--md-nav__sticky-zindex);

The fact that it has an inline style attribute on the element makes me think this was manipulated in JS tough. A quick search for --md-nav__sticky-zindex And I found the rule

sphinx-immaterial/src/assets/stylesheets/main/layout/_nav.scss

Lines 185 to 190 in 4eb8655

	&__sticky {
	position: sticky;
	top: var(--md-nav__header-height, 0);
	z-index: var(--md-nav__sticky-zindex);
	background-color: var(--md-default-bg-color);
	}

Sure enough, the style is set in the JS implementation for toc.sticky, but it may be neglecting the toc title.

sphinx-immaterial/src/assets/javascripts/components/toc/index.ts

Line 407 in 4eb8655

link.style.setProperty("--md-nav__sticky-zindex", zindex.toString())

This is tricky because I'm not the best at JS, and the toc is structured as lists of invisible checkboxes in the HTML. I have a feeling it is an easy fix though.

[2bndy5]

I think this actually calls for a CSS fix concerning the viewport's min-width. Looking again at the dev console and filtering the rules to show anything about z-index, I see the z-index: var(--md-nav__sticky-zindex); is overridden by

sphinx-immaterial/src/assets/stylesheets/main/layout/_nav.scss

Lines 678 to 681 in 4eb8655

	> .md-nav__link {
	position: sticky;
	top: 0;
	z-index: 1;

[2bndy5]

This was my bad. The last update from upstream introduced this z-index override. I think we'll be ok to simply remove it.

[2bndy5]

@mhostetter when you get a chance, can you verify if the fix-189 branch resolves this?

[mhostetter]

@2bndy5 yes, it works! Thanks for the speedy response and fix, as always!

[jbms]

Before reverting that upstream change, we should probably check why it was added in the first place and confirm that removing it won't cause any problems.

Also if we do remove it, we should add a sphinx-immaterial comment.

[2bndy5]

The upstream commit (no PR related) just says "merge features related to scotch bonnet funding goal". I'm inclined to think it has something to do with how the toc.follow feature that was implemented upstream. However, I didn't use the JS related to that upstream implementation (in favor of our own implementation).

There are other z-index rules that refer to safari compatibility (in an similar bug report upstream):

sphinx-immaterial/src/assets/stylesheets/main/layout/_nav.scss

Lines 521 to 526 in 4eb8655

	&--secondary &__title {
	position: sticky;
	top: 0;
	// Hack: because of the hack that we need to make .md-ellipsis work in
	// Safari, we need to set `z-index` here as - see https://bit.ly/3s5M2jm
	z-index: 1;

sphinx-immaterial/src/assets/stylesheets/main/layout/_nav.scss

Lines 547 to 552 in 4eb8655

	&--primary &__title {
	position: sticky;
	top: 0;
	// Hack: because of the hack that we need to make .md-ellipsis work in
	// Safari, we need to set `z-index` here as - see https://bit.ly/3s5M2jm
	z-index: 1;

both of which also came with #171

I still think we're ok to remove it (and replace with a comment) since our implementation for toc.follow is a bit different. My second approach is to add a special rule to override this regression for the specific viewport width, but I'm not sure what the size difference is between break-from-device(screen) and break-from-device(tablet landscape) [found the definitions in _config.scss]. I also don't have access to a mac to test changes on the safari browser.

[2bndy5]

Instead of removing the z-index: 1 rules, I think we can retain the intended functionality by using the var with a default fallback value of 1 (z-index: var(--md-nav__sticky-zindex, 1)). The variable --md-nav__sticky-zindex is only set (via inline style attributes) on elements from the JS that implements the toc.follow feature.

[jbms]

Instead of removing the z-index: 1 rules, I think we can retain the intended functionality by using the var with a default fallback value of 1 (z-index: var(--md-nav__sticky-zindex, 1)). The variable --md-nav__sticky-zindex is only set (via inline style attributes) on elements from the JS that implements the toc.follow feature.

This sounds good to me.

[2bndy5]

@mhostetter It would help if you could re-test the fix-189 branch since the CSS solution has slightly changed.

[mhostetter]

@2bndy5 It works for me, thanks!