Wp-calypso: Notifications: List blocks are poorly rendered.

[kwight]

The block editor supports ordered and unordered blocks, both capable of having multiple indentation levels. Only top-level unordered lists are somewhat understandable in notifications – nothing else is.

Front-End | Notifications
--- | ---
|

[blackjackkent]

Spent some time looking at this yesterday/today and I have to say I'm a little stumped on this one. 🤔 This is more complicated than the other HTML elements we've looked at remedying in web notifications; the current behavior right now is that the API registers the location of the start of the list and end of the list and strips out all li elements, replacing them with bullet points and newline separators:

if ( $element->parentNode->nodeName === "li" ) {
    if ( $element->isSameNode( $element->parentNode->firstChild ) ) {
        // We only want a • prepended if this is the first thing after a <li>
        $text_content = "• " . $text_content;
    }
    if ( $element->isSameNode( $element->parentNode->lastChild ) ) {
        // We only want a \n appended if this is the last thing before a </li>
        $text_content = $text_content . "\n";
    }
}

This is then parsed recursively on the Calypso receiving end, except that the code doesn't have any way of knowing, once it is parsing the values, what nested level each item should be at and immediately closes off each list item rather than allowing it to have children:

// detect list items
//  - one
//  * two
//  [bullet] three
//  [dash] four
if ( /^\s*[*\-\u2012-\u2015\u2022]\s/.test( coded ) ) {
    return {
        out:
            out +
            ( inList ? '' : '<ul class="wpnc__body-list">' ) +
                <li>${ coded.replace( /^\s*[*\-\u2012-\u2015\u2022]\s/, '' ) }</li>`,
        inFence,
        inList: true,
    };
}

This is pretty fragile and breaks down as soon as a particular li needs to remain open to allow a child element. (It also completely ignores the possibility of ordered lists.)

I'm not really sure what the best way to address this is but it's going to require a more significant rework than the previous notification tickets have. Depending how urgent this is considered, I might switch to working on some Earn tickets and percolate on this a little before circling back.

[kwight]

the API registers the location of the start of the list and end of the list and strips out all li elements, replacing them with bullet points and newline separators

This must have made sense at one point, but I don't think it does any more – it sounds like if we stripped out this custom handling, and just whitelisted the related HTML tags, we could solve ourselves a lot of trouble and end up with the best support possible to boot. @blackjackkent what do you think?

[blackjackkent]

@kwight Yeah, I think you might be right; I was giving it some more thought last night. Certainly I don't think the current logic is necessary. This is definitely still going to require more work than the other markup issues but mostly because it will entail digging out the existing logic and replacing it.

I'll poke more at this tomorrow.

[blackjackkent]

Made good progress on this today. I've got the HTML generating correctly; need to resolve some lingering CSS issues and will have a PR up tomorrow.