ENH: Collapse only in doc gen

[larsoner]

Closes #12130
Closes #12131
Closes #12149

If we eventually want a way to control this behavior we could add env var, context manager, etc. but not implemented here (YAGNI from #12131). Locally seems to work:

[hoechenberger]

Looks quite good to me! 😍

[larsoner]

Appears to have worked on CircleCI. @drammock feel free to review/merge if you're happy

[hoechenberger]

I'm sill seeing #12130 in the doc link you posted, btw
This is with latest Safari on macOS

[larsoner]

Argh, not on Chrome on Linux:

I'll remove the closes for that issue and re-title since now it's a doc building issue not a Report issue

[drammock]

Argh, not on Chrome on Linux:

firefox is also OK. I think this is Safari misbehaving. MDN docs say:

Safari treats visibility: collapse like hidden, leaving a white gap.

and (though not relevant here)

Safari supports the collapse value only on <tr>, <thead>, <tbody>, and <tfoot>, but not on <col> and <colgroup> elements.

[hoechenberger]

@drammock If I read the specs correctly, what Chrome and FF do is an implementation detail and Safari is following the standard too

For other elements, collapse is treated the same as hidden.

I think we'll need to find a different solution to do the collapsibles if we want to ensure interoperability

[hoechenberger]

How abou trying:

<details>
    <summary>...</summary>
    ...
</details>

[drammock]

If I read the specs correctly, what Chrome and FF do is an implementation detail and Safari is following the standard too

I think you're mistaken; the elements in question are tr elements. In the spec for collapse it says:

For <table> rows, columns, column groups, and row groups, the row(s) or column(s) are hidden and the space they would have occupied is removed (as if display: none were applied to the column/row of the table).

EDIT: in other words, the part you quoted about "other elements" doesn't apply; these aren't "other" elements they're table elements.

[hoechenberger]

Ah, that makes sense!!

In any case, it's not working as expected in Safari 😢

[hoechenberger]

Yes, <details> does exactly what we need it seems
and requires much less code

I put together this info.html.jinja to test:

<details>
    <summary>General!</summary>
    <table class="table table-hover table-striped table-sm table-responsive small">
        <tr>
            <th class="collapsible_header" colspan="2" style="font-weight: bold; text-align: left;">
                <label for={{ section_ids[0] }}>
                    {{sections[0]}}
                </label>
            </th>
        </tr>
        <tr class="{{ section_ids[0] }}">
            <th>Measurement date</th>
            {% if meas_date is not none %}
            <td>{{ meas_date }}</td>
            {% else %}
            <td>Unknown</td>
            {% endif %}
        </tr>
        <tr class="{{ section_ids[0] }}">
            <th>Experimenter</th>
            {% if experimenter is not none %}
            <td>{{ experimenter }}</td>
            {% else %}
            <td>Unknown</td>
            {% endif %}
        </tr>
        <tr class="{{ section_ids[0] }}">
            <th>Participant</th>
            {% if subject_info is defined and subject_info is not none %}
            {% if 'his_id' in subject_info.keys() %}
            <td>{{ subject_info['his_id'] }}</td>
            {% endif %}
            {% else %}
            <td>Unknown</td>
            {% endif %}
        </tr>
    </table>
</details>

MWE:

# %%
import mne

sample_dir = mne.datasets.sample.data_path()
sample_fname = sample_dir / "MEG" / "sample" / "sample_audvis_raw.fif"

raw = mne.io.read_raw_fif(sample_fname, preload=True)
raw.crop(tmax=60)

raw.info

# %%
report = mne.Report()
report.add_raw(raw, title="Raw")
report.save("/tmp/foo.html", overwrite=True)

Jupyter:

Report:

But we'd still need a way to control the initial collapse state, I don't have time for any further investigations today (it's bed time for me)

(You can't see it in the screenshots but the vertical space is cleared.)

[drammock]

Yes, <details> does exactly what we need it seems
and requires much less code

nice solution! should be straightforward to do the same jinja if/else trick with the open attribute of details that @larsoner did with the checked attribute of label

[hoechenberger]

We just need to ensure it keeps working in both Jupyter and the docs … fingers crossed

[larsoner]

@drammock feel free to push a commit if you want, I won't have time to look until tonight at the earliest (and maybe next week)

[hoechenberger]

the open attribute of details

Nice find, this should do the trick!

[larsoner]

This is the result. It changes it to separate tables but I guess that's okay.

... and then I realized that there's a bug with the "number of good channels" logic -- there should be 203 grad and 59 EEG listed as "Good channels". I wonder if it would be better to say "All channels" here (?) but the intention seems pretty clear based on the row label:

Pushing a fix plus test.

[larsoner]

And from CircleCI, the default:

And with some clicking:

[hoechenberger]

Great!
@larsoner I asked my editor to "fix" the indentation in bc489e6
Feel free to revert if you don't like it

[larsoner]

Added curlylint. It necessitated adding stuff like this, which I actually don't mind -- it helps us document a bit what HTML tags we have opened and/or will close later.

[larsoner]

Had to comment this one out because we are not consistent. A follow-up PR could uncomment this and fix all files, and we could add it to .gitignore.

[hoechenberger]

very nice!!

[drammock]

failures are Jinja-related here: jinja2.exceptions.TemplateSyntaxError: expected token 'end of print statement', got 'integer'. Failing in

FAILED mne/commands/tests/test_commands.py::test_report
FAILED mne/report/tests/test_report.py::test_render_report[pyvistaqt]
FAILED mne/report/tests/test_report.py::test_render_report_extra[pyvistaqt]
FAILED mne/report/tests/test_report.py::test_add_custom_css
FAILED mne/report/tests/test_report.py::test_add_custom_js
FAILED mne/report/tests/test_report.py::test_render_non_fiff
FAILED mne/report/tests/test_report.py::test_render_add_sections[pyvistaqt]
FAILED mne/report/tests/test_report.py::test_render_mri[pyvistaqt]
FAILED mne/report/tests/test_report.py::test_render_mri_without_bem
FAILED mne/report/tests/test_report.py::test_multiple_figs
FAILED mne/report/tests/test_report.py::test_scraper
FAILED mne/report/tests/test_report.py::test_survive_pickle
FAILED mne/report/tests/test_report.py::test_manual_report_2d
FAILED mne/report/tests/test_report.py::test_manual_report_3d[pyvistaqt]
FAILED mne/report/tests/test_report.py::test_sorting

[drammock]

there was a jinja error ({{ instead of {% around the if statements) which I've fixed in 213e95c. But curlylint now fails, it apparently won't allow closing an tag inside a conditional if the tag was opened outside the conditional: thibaudcolas/curlylint#46.

[drammock]

given that curlylint is described as "experimental", is causing us to add extra code that isn't run, and is still failing, EDIT: and its most recent commit was 1.5 years ago, I think we need to either put it on the shelf until it's more mature, or at minimum make its pre-config test always pass and just print the warnings --- though I'm not a fan of this latter option, since it will print this every time:

Details

mne/html_templates/report/footer.html.jinja
6:3     Parse error: expected one of 'autoescape', 'block', 'blocktrans', 'comment', 'filter', 'for', 'if', 'ifchanged', 'ifequal', 'ifnotequal', 'not an intermediate Jinja tag name', 'spaceless', 'verbatim', 'with' at 6:3      parse_error

mne/html_templates/report/header.html.jinja
58:1    Parse error: expected one of '[:a-zA-Z]', 'animate', 'animateMotion', 'animateTransform', 'area', 'base', 'br', 'circle', 'col', 'ellipse', 'embed', 'feBlend', 'feColorMatrix', 'feComposite', 'feConvolveMatrix', 'feDisplacementMap', 'feDistantLight', 'feDropShadow', 'feFlood', 'feFuncA', 'feFuncB', 'feFuncG', 'feFuncR', 'feGaussianBlur', 'feImage', 'feMergeNode', 'feMorphology', 'feOffset', 'fePointLight', 'feSpotLight', 'feTile', 'feTurbulence', 'hr', 'image', 'img', 'input', 'line', 'link', 'meta', 'mpath', 'param', 'path', 'polygon', 'polyline', 'rect', 'script', 'set', 'source', 'stop', 'style', 'track', 'use', 'wbr', '{#', '{%', '{{' at 58:1  parse_error

mne/html_templates/report/toc.html.jinja
14:5    Parse error: expected one of '[:a-zA-Z]', 'animate', 'animateMotion', 'animateTransform', 'area', 'base', 'br', 'circle', 'col', 'ellipse', 'embed', 'feBlend', 'feColorMatrix', 'feComposite', 'feConvolveMatrix', 'feDisplacementMap', 'feDistantLight', 'feDropShadow', 'feFlood', 'feFuncA', 'feFuncB', 'feFuncG', 'feFuncR', 'feGaussianBlur', 'feImage', 'feMergeNode', 'feMorphology', 'feOffset', 'fePointLight', 'feSpotLight', 'feTile', 'feTurbulence', 'hr', 'image', 'img', 'input', 'line', 'link', 'meta', 'mpath', 'param', 'path', 'polygon', 'polyline', 'rect', 'script', 'set', 'source', 'stop', 'style', 'track', 'use', 'wbr', '{#', '{%', '{{' at 14:5  parse_error

Oh no! 💥 💔 💥
3 errors reported

[drammock]

OK I've purged curlylint; should get the CIs green again.

[larsoner]

EDIT: and its most recent commit was 1.5 years ago

😱 thanks @drammock agreed it's not worth keeping around yet

[drammock]

here's the rendered tutorial, still looks good (auto-collapsed) https://output.circle-artifacts.com/output/job/72aeac1a-9296-4049-abd5-529cd83e85b8/artifacts/0/html/auto_tutorials/intro/30_info.html#dropping-channels-from-an-info-object