Handle Pandoc change in table classes #10906
Conversation
|
Test passes locally. But I should have guessed that first try with this feature would not work on CI. Probably this need to be OS specific. I'll debug this. |
|
We need to have a longer conversation about this. The filter you suggested is a good idea, except (as I understand it) that it will trigger on all tables, not just those that Pandoc itself parsed, and that is a problem with (eg) our html table processing. |
Then it is on me and my incorrect understanding of how our filter chain works. I specifically added early in normalize step so that it applies first in AST for any To be clear, my intention is to apply specifically on Pandoc parsed Table only, and avoid our parsed HTML table. And I did as a draft so that we have a base to discuss on. So 💯 for discussion. 👍 |
Nope, that's on me, I wrote too fast! |
This reverts part of commit dd6717f. The `.table` is set by quarto itself and its removal in html output is a sign that something has changed and it is not set correctly anymore. We restore original test to correctly fix this.
It has its own loading different than main.lua one.
cderv
force-pushed
the
fix/table-style-new-pandoc
branch
4 times, most recently
from
cedf9c5
to
84cb8dc
Compare
cderv
force-pushed
the
fix/table-style-new-pandoc
branch
from
84cb8dc
to
e0b526b
Compare
CI needs to be adapted to correctly run visual tests.
|
Regarding visual testing with playwright, it requires an update in our GHA workflows so that we can have working tests. Right now, it works locally but we have pixels differences between local and CI, even on same OS (linux for example). So I'll handle the update of CI in another workflow. Maybe it will be easier to run playwright tests, out of smoke-all runs and parallel testing to simplify the conditional and configuration. |
This makes more sense to get back the behavior from previous Pandoc.
|
@cscheid following our call, I have updated to do the change as HTML fixup in post render to match more closely previous Pandoc's behavior |
|
This is excellent too. |
Follow up on #10906 which was incomplete
Fix #10904
Close #10607
This reverts part of commit dd6717f.
The
.tableis set by quarto itself and its removal in html output is a sign that something has changed and it is not set correctly anymore.We restore original test to correctly fix this.
See details explanation of current situation at
Opening this PR as a draft with first idea to put back the odd, even and header class on tables that are parsed directly by Pandoc.
We could also find other solution with new approach to serve the same purpose (identifying pandoc's table to style them)
This PR includes snapshot test with playwright so that we can detect next time when the look of the tables are changing.
(BTW this is a first version of snapshot testing with playwright - it could be quite heavy to commit each png, so we could also have something different by leveraging caching in CI: (1) git ignore the
__screenshot__folder, and (2) cache it on CI so that each news runs can access the snapshot. )