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

Get CI audits back to passing #455

Closed
bollwyvl opened this issue Aug 27, 2021 · 5 comments · Fixed by #456
Closed

Get CI audits back to passing #455

bollwyvl opened this issue Aug 27, 2021 · 5 comments · Fixed by #456
Assignees

Comments

@bollwyvl
Copy link
Collaborator

Somewhere between #430 and #425 we regressed on the lighthouse scores, and #294 has raised the bar higher. Let's investigate getting these back to passing, or failing that, loosening the required metrics.

@bollwyvl bollwyvl self-assigned this Aug 27, 2021
@bollwyvl
Copy link
Collaborator Author

Initial finding:

@bollwyvl
Copy link
Collaborator Author

Here is a relatively ghetto way to get something in there...

     # overwritten
    def visit_title(self, node: Element) -> None:
        if isinstance(node.parent, addnodes.compact_paragraph) and node.parent.get('toctree'):
            node_level = 1
            parent = node.parent
            while not (
                parent is None or
                isinstance(parent, addnodes.document)
            ):
                parent = parent.parent
                node_level += 1
            #__import__('pprint').pprint(node.parent.parent.__dict__)
            self.body.append(self.starttag(node, 'p', '', CLASS='caption', ROLE='heading', **{'ARIA-LEVEL': node_level}))
            self.body.append('<span class="caption-text">')
            self.context.append('</span></p>\n')
        else:
            super().visit_title(node)
        self.add_secnumber(node)
        self.add_fignumber(node.parent)
        if isinstance(node.parent, nodes.table):
            self.body.append('<span class="caption-text">')

@choldgraf
Copy link
Collaborator

choldgraf commented Aug 27, 2021

hmmm - so this is a regression based on core Sphinx? I don't think that we should write lots of hacky code to un-regress core Sphinx or this is going to be a very painful cat and mouse game. Is there a fix that should be made in Sphinx?

And it sounds like the main problem is that aria-level is not in there...is that right?

Maybe a less hacky way to do this (well, still hacky, but less code) would be to just grab the soup object around here:

https://github.com/pydata/pydata-sphinx-theme/blob/master/pydata_sphinx_theme/__init__.py#L64

and then do something like

for ii in soup.findAll(ROLE="heading"):
  if "aria-level" not in ii.attrs:
    figure-out-level-and-add-it

@bollwyvl
Copy link
Collaborator Author

so this is a regression based on core Sphinx

Yeah, it landed in 4.1.

a very painful cat and mouse game

Yep.

and then do something like...

That might be easier in the near term.... though, as mentioned, captions like this show up in both the toc and next to figures.

@jorisvandenbossche
Copy link
Member

@bollwyvl thanks for looking into this! One thing I am wondering: since it's a regression in sphinx, has this already been reported over there as well?

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 a pull request may close this issue.

3 participants