Use semantic markup in flyout

[jonels-msft]

Fixes #953

I've always had a bit of a soft spot for <dl>, but in the case of our flyout menu it confuses screen readers. Semantically what we have is a set of headers describing lists of items, so that's the markup I used.

[Blendify]

I would use a more specific CSS selector here such as a "label" class.

[jonels-msft]

@Blendify I added the label to html and style. See if it looks good.

[Blendify]

Looks good but I want confirmation from @agjohnson that this won't have any issue with read the docs itself

[stsewd]

Change looks good, this doesn't break rtd.org, but it won't be visible in projects from .org, since we override that section with https://github.com/readthedocs/readthedocs.org/blob/master/readthedocs/api/v2/templates/restapi/footer.html. Having that change in the repo I just linked would be ideal, but it may break users using some customization around, since it's changing the tags. We are working to return a json with the data instead of html so themes can customize this better.

[jonels-msft]

@stsewd would you consider, as an intermediate step between the current situation and the JSON footer, having a flag in RTD to toggle the markup in restapi/footer.html? By default it could stay what it is, but through a configuration parameter it could switch to being more accessible?

[ericholscher]

Yea, I think we could improve the content on the RTD-injected footer, and set it future_default_true so all new projects get it. We could also opt old projects into it on request. This is how we accomplish a lot of breaking changes that will impact older users.

[agjohnson]

Didn't get a chance to take a deep look at this, but this is a strong breaking change for downstream usage of the theme. Many of our users and customers have worked around this CSS to hide these elements unfortunately, so it makes this change pretty costly.

If you need this in your docs immediately, it might be best as a local override of the theme in your own theme.

[ericholscher]

We talked about this internally; I didn't realize this was changing the structure of the footer. That's a much larger change, and something that will effect a lot of downstream users, so it's something that we'll need to do a lot more work to support, at least on the RTD side of things. If there's a way to get more accessible options for the existing markup, that's something we could push out quickly -- but changing the actual HTML is going to be much harder.

We're actually going to be moving to a data-driven approach to the footer, which will avoid this problem (readthedocs/readthedocs.org#8052) -- but that work isn't on our near-term roadmap currently.

[jonels-msft]

If I'm understanding the situation correctly, it won't be possible to fix this accessibility problem on RTD-hosted sites until readthedocs/readthedocs.org#8052 goes live? (Unless there's a way to decorate the existing markup without substantially changing it.)

That is, if I cherry-pick this PR to my fork of the theme and use the forked theme in https://docs.citusdata.com , will RTD still override my changes with the <dl> based markup?

[agjohnson]

@jonels-msft Yeah, that's mostly correct. We do need to design and develop and API return for themes to consume first. The issue is that we can't easily change the DOM structure injected by our application because downstream users and customers use custom stylesheets based on the existing DOM structure output from our application.

The part that makes this expensive is we would need to support multiple DOM structures from our application. Instead, we have decided a while back that we want to remove generation of this DOM structure from our application entirely, so this would be going in the opposite direction.

It is however possible to hard code a versions.html and to avoid our injection of this menu. We inject this HTML here: https://github.com/readthedocs/readthedocs.org/blob/master/readthedocs/core/static-src/core/js/doc-embed/footer.js#L11-L15

That is, if you are using a derivative theme of sphinx_rtd_theme, our client will replace div.rst-other-versions with the generated content. On your custom versions.html, you might be able to just remove this class and avoid the injected menu. This will require your team experiment a bit however, I haven't tested this.