sphinx_rtd_theme
sphinx_rtd_theme copied to clipboard
Work around Firefox list marker positioning issue
Firefox displayed list markers too low when the first element in a list element is a literal block. Removing the overflow property from the pre element works around this. The overflow property seems to be redundant, as the highlight div wrapping the pre also has overflow-x set to display a scroll bar when necessary, and overflow-y doesn't seem to have any use here.
I'm not quite sure what is going on here, but I have not found a reason why
this overflow property exists, so I'm proposing to remove it. There is a similar
property on the pre.literal-block
selector, but I haven't found out what the
literal-block
class is used for at all, so I didn't touch it.
Before:
After:
Any chance we can get someone to have a look at the change?
Ping, another 2 months have passed.
Sorry for the late reply @NeoRaider and thank you for looking into this issue.
There are several PRs waiting to be merged addressing the overflow
property on pre
tags in different situations from this, so I don't think it's safe to implement this at the moment.
Also, it does not feel right to address a vertical alignment issue by removing an overflow. I fear there would be unforeseen side effects.
I would be 👍 with adding a new selector to address this specific issue. Can you test if adding the lines bellow to the _theme_rst.sass file instead solves the problem?
.rst-content ul pre
display: inline-block !important
vertical-align: middle
If it works, feel free to update this PR. Or I can add it in a future PR.
Thanks for the reply.
Making this an inline block doesn't seem like a step in the right direction:
The whole highlighted block is now considered a single line, with the bullet aligned in the middle. I can move the bullet to the top using vertical-align: top
, but it doesn't align correctly with the first line of the highlighted block anymore, like it does when the overflow is set to visible
.
I'm somewhat at a loss why the overflow: auto
on the pre
element exists in the first place. The surrounding <div class="highlight>
sets overflow-x: auto
(and I've confirmed that vertical scrolling is still working fine with my patch), and I'm not aware of any situations where overflow-y
would be relevant for highlighted blocks.
I see your point with the overflow: auto
rule being redundant, but I still don't think that the risk of creating worse problems with the side effects of removing it is worth for now. I'll revisit this after the 1.1 release PRs are merged.
I see your point with the
overflow: auto
rule being redundant, but I still don't think that the risk of creating worse problems with the side effects of removing it is worth for now. I'll revisit this after the 1.1 release PRs are merged.
Updated PR to latest master. I verified that the issue still exists with the latest versions of the theme and Firefox, and that this PR still fixes it.
CSS issue this was working around was fixed at some point between Firefox 102 and 108, closing.
Thanks for revisiting and updating :100: