FancyThemes / wp-advanced-excerpt

A WordPress plugin that allows you to control the appearance of post excerpts
GNU General Public License v3.0
23 stars 7 forks source link

Ellipsis and Read-More text not appearing at the end of excerpt #22

Closed aprea closed 10 years ago

aprea commented 10 years ago

Example:

read-more pnmg

Seems to be the case when tables / other markup is involved.

aprea commented 10 years ago

Yo @bradt, thought I'd run this one by you. If I "fix" this problem I'm going to be changing the functionality of the plugin.

Here's what the current function looks like:

public function text_add_more( $text, $ellipsis, $read_more ) {
    if ( $read_more ) {
        $link_template = apply_filters( 'advanced_excerpt_read_more_link_template', ' <a href="%s" class="read-more">%s</a>', get_permalink(), $read_more );
        $ellipsis .= sprintf( $link_template, get_permalink(), $read_more );
    }

    $pos = strrpos( $text, '</' );
    if ( $pos !== false ) {
        // Inside last HTML tag
        $text = substr_replace( $text, $ellipsis, $pos, 0 );
    } else {
        // After the content
        $text .= $ellipsis;
    }

    return $text;
}

More importantly though is this code:

    $pos = strrpos( $text, '</' );
    if ( $pos !== false ) {
        // Inside last HTML tag
        $text = substr_replace( $text, $ellipsis, $pos, 0 );
    } else {
        // After the content
        $text .= $ellipsis;
    }

It attempts to find the last HTML tag in the excerpt and place the ellipsis / read more text inside of that tag.

This works fine for tags like <div>, <p> etc. But produces weird results with <table> tags.

There's probably others that also produce weird results.

My suggestion - We maintain a list of HTML elements that aren't compatible with appends and instead place the ellipsis / read more text after these elements (i.e. the else statement from above).

You have a better / more elegant solution?

bradt commented 10 years ago

Sounds like a good solution to me. I think it might be easier the other way around. That is, maintain a list of HTML elements where you should append the ellipsis / read more text inside. I think it might be a shorter list. Take a look for yourself and decide what's best.

bradt commented 10 years ago

@aprea Ready for review?

aprea commented 10 years ago

Needs some better code comments, once I add that then it'll be ready for review.

aprea commented 10 years ago

Code comment added, ready for review.

https://github.com/deliciousbrains/wp-advanced-excerpt/commit/0f19580caaebb3595ea100115dc2680665d1a0dd

bradt commented 10 years ago

Looks good.