Minor issue with nested lists
javiereguiluz opened this issue · 10 comments
In this file -> https://github.com/EasyCorp/EasyAdminBundle/blob/master/doc/events.rst we have this:
All events are triggered using objects instead of event names defined as strings
(as recommended since Symfony 4.3). They are defined under the
``EasyCorp\Bundle\EasyAdminBundle\Event\`` namespace:
* Events related to Doctrine entities:
* ``AfterEntityBuiltEvent``
* ``AfterEntityDeletedEvent``
* ``AfterEntityPersistedEvent``
* ``AfterEntityUpdatedEvent``
* ``BeforeEntityDeletedEvent``
* ``BeforeEntityPersistedEvent``
* ``BeforeEntityUpdatedEvent``
* Events related to resource admins:
* ``AfterCrudActionEvent``
* ``BeforeCrudActionEvent``This is the right way to create nested lists for Sphinx (so I guess it's the right way in RST in general too). That includes the blank lines after the first level bullets. In the current parser it works OK, but in the new parser it generates this wrong HTML:
As you can see, the nested <ul> is outside of the <li>, but it should be inside of it.
I can confirm the issue.
Yes, I'm working on it while also fixing https://github.com/weaverryan/docs-builder/issues/38 . Unfortunately, lists is implemented completely differently from definition lists, so I first have to investigate if we can safely rewrite it to be the same.
doctrine/rst-parser#143 is now merged, this should be fixed now.
It works great! Thank you.
Although the main problem was fixed, there's still a minor thing to consider.
Before, nested lists were parsed like this:
<ul class="...">
<li>
Events related to Doctrine entities:
<ul>
<li><code class="...">AfterEntityBuiltEvent</code></li>
<li><code class="...">AfterEntityDeletedEvent</code></li>
<li><code class="...">AfterEntityPersistedEvent</code></li>
<!-- ... -->
</ul>
</li>
<!-- ... -->
</ul>Now they are parsed like this:
<ul>
<li>
<p>Events related to Doctrine entities:</p>
<ul>
<li><code class="...">AfterEntityBuiltEvent</code></li>
<li><code class="...">AfterEntityDeletedEvent</code></li>
<li><code class="...">AfterEntityPersistedEvent</code></li>
<!-- ... -->
</ul>
</li>
<!-- ... -->
</ul>The <p> introduces some visual changes. Do we want to change this and not generate the <p> element? ... or is it more correct to have that <p> and then we should update the styling? Thanks!
Hi @javiereguiluz! I followed the behavior of Python Sphinx here: if there is only 1 paragraph in a list item, the <p> tag is omitted. Otherwise, all tags are preserved.
We can change it as we wish, but as it's a generic library I want to stay close to Sphinx. Optionally, if we finally implement node visitors, we might be able to make these tweaks in this library.
OK, but symfony.com, which still uses Sphinx, doesn't show the <p> in the docs (see https://symfony.com/doc/current/bundles/EasyAdminBundle/events.html). Maybe I'm missing something. Thanks.
Oh, that's interesting. I'll check somewhere today with the rst-parser test suite
@javiereguiluz I cannot reproduce locally using Sphinx 1.8.5 (which should be the version used on symfony.com):
$ git clone https://github.com/doctrine/rst-parser && cd rst-parser
$ sphinx-build --version
sphinx-build 1.8.5
$ ./tests/sphinx list
$ grep -A6 'sub items' tests/Functional/_sphinx/list.html
<li><p class="first">This is an item with sub items</p>
<ul class="simple">
<li>A valid</li>
<li>subitem</li>
<li>list</li>
</ul>
</li>
OK, let's close this because Wouter explained the tech reasons behind this ... and we've tweaked the design a bit to neutralize this change. Thanks!
