Make WordPress Core

Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#30135 closed enhancement (fixed)

Twenty Fifteen: Replaces ellipses with a Continue reading link.

Reported by: davidakennedy's profile davidakennedy Owned by: iandstewart's profile iandstewart
Milestone: 4.1 Priority: normal
Severity: normal Version: 4.1
Component: Bundled Theme Keywords: has-patch commit
Focuses: accessibility Cc:

Description

To help meet accessibility-ready requirements, we need to make clean automatically-generated excepts. We should replace the ellipses (appended to automatically generated excerpts) with a Continue reading link.

This would mirror what we already do in the content.php and other content areas, and is catching another use-case:

<?php
    /* translators: %s: Name of current post */
    the_content( sprintf(
        esc_html__( 'Continue reading %s', 'twentyfifteen' ),
            the_title( '<span class="screen-reader-text">', '</span>', false )
        ) );
?>

We could add it in the template-tags.php file and follow this example: http://www.fklein.info/read-more-link-excerpts/

Attachments (6)

30135.patch (1.1 KB) - added by davidakennedy 9 years ago.
30135.2.patch (1.5 KB) - added by kraftbj 9 years ago.
Disables the filter when is_admin
30135.3.patch (3.2 KB) - added by davidakennedy 9 years ago.
30135.4.patch (2.8 KB) - added by davidakennedy 9 years ago.
30135.5.patch (2.8 KB) - added by davidakennedy 9 years ago.
30135.6.patch (2.9 KB) - added by davidakennedy 9 years ago.
Removes leading space; concatenates ellipses with spaces and link.

Download all attachments as: .zip

Change History (29)

@davidakennedy
9 years ago

#1 @davidakennedy
9 years ago

  • Keywords has-patch needs-testing added

This ticket was mentioned in Slack in #accessibility by davidakennedy. View the logs.


9 years ago

#3 @sharonaustin
9 years ago

  • Keywords needs-testing removed

Tested the patch, and works as intended. Thank you, David!

Last edited 9 years ago by sharonaustin (previous) (diff)

#4 @rianrietveld
9 years ago

Hi David, tested the patch and it works, nice job!

This ticket was mentioned in Slack in #accessibility by rianrietveld. View the logs.


9 years ago

#6 @michaelbeil
9 years ago

Hey David, the patch works well.

#7 @iamtakashi
9 years ago

Don't we want ellipses before the "Continue reading" link? In some cases, it reads weird :)

https://cldup.com/Ab3gQPsv-g.png

#8 @kraftbj
9 years ago

On wp-admin/edit.php, when using the "excerpt" display mode, this filter is adding "Continue Reading" links to the admin. I suggest an ! is_admin() or similar check to keep the admin interface clean.

https://cldup.com/i9aU2Fwpgb.png

Patch forthcoming (without yet an ellipses as suggested by iamtakashi)

Related: #30026 (same thing in Twenty Ten/Eleven)
Related: #30024 (add Continued Reading links to manual excerpts, closed as wontfix currently)

Last edited 9 years ago by kraftbj (previous) (diff)

@kraftbj
9 years ago

Disables the filter when is_admin

#9 follow-up: @iamtakashi
9 years ago

Also, we don't want the arrow. Having more-link class should match with other continue reading links.

#10 in reply to: ↑ 9 @davidakennedy
9 years ago

Replying to iamtakashi:

Also, we don't want the arrow. Having more-link class should match with other continue reading links.

Great catches, @iamtakashi and @kraftbj!

I've updated the patch to include @kraftbj's changes and to remove the right arrow in favor of Genericon approach.

#11 follow-up: @davidakennedy
9 years ago

This time, a patch without the press-this.php stuff. Sorry about that.

#12 in reply to: ↑ 11 ; follow-up: @iamtakashi
9 years ago

Replying to davidakennedy:

This time, a patch without the press-this.php stuff. Sorry about that.

What about having ellipsis before the link?

This ticket was mentioned in Slack in #accessibility by michael. View the logs.


9 years ago

This ticket was mentioned in Slack in #accessibility by michael. View the logs.


9 years ago

#15 in reply to: ↑ 12 @davidakennedy
9 years ago

Replying to iamtakashi:

Replying to davidakennedy:

This time, a patch without the press-this.php stuff. Sorry about that.

What about having ellipsis before the link?

Sorry @iamtakashi! Working on too many similar patches. I will get this right eventually. :)

#16 follow-up: @iamtakashi
9 years ago

Someone can know more about it but, would the leading space before the ellipsis be concerned? That string is not for translation so it might be ok but it weirds me out a bit.

Maybe, we should wrap the ellipsis with a span and give it space with css?

#17 in reply to: ↑ 16 ; follow-up: @davidakennedy
9 years ago

Replying to iamtakashi:

Someone can know more about it but, would the leading space before the ellipsis be concerned? That string is not for translation so it might be ok but it weirds me out a bit.

Maybe, we should wrap the ellipsis with a span and give it space with css?

Looks like we shouldn't do that. See: http://codex.wordpress.org/I18n_for_WordPress_Developers#Best_Practices

I'll do some research and adjust the patch.

#18 in reply to: ↑ 17 @SergeyBiryukov
9 years ago

Replying to davidakennedy:

Looks like we shouldn't do that. See: http://codex.wordpress.org/I18n_for_WordPress_Developers#Best_Practices

That only applies to translatable strings.

The leading space before the ellipsis looks weird to me too. Could we append the "Continue reading" link to the passed $more argument? Or just remove the space?

@davidakennedy
9 years ago

Removes leading space; concatenates ellipses with spaces and link.

This ticket was mentioned in Slack in #core-themes by iandstewart. View the logs.


9 years ago

#20 @iandstewart
9 years ago

  • Keywords commit added

#21 @iandstewart
9 years ago

  • Milestone changed from Awaiting Review to 4.1

#22 @iandstewart
9 years ago

  • Owner set to iandstewart
  • Resolution set to fixed
  • Status changed from new to closed

In 30237:

Twenty Fifteen: use continue reading links for auto-generated excerpts to provide a readable link for screenreaders.

Props davidakennedy, kraftbj, fixes #30135.

#23 @grapplerulrich
9 years ago

@davidakennedy - Why did you wrap add_filter( 'excerpt_more', 'twentyfifteen_excerpt_more' ); within the if ( ! function_exists( 'twentyfifteen_excerpt_more' )?

The reason I ask is that is that add_action( 'after_setup_theme', 'twentyfifteen_setup' ); is kept outside the check. https://github.com/WordPress/WordPress/blob/master/wp-content/themes/twentyfifteen/functions.php#L122

Note: See TracTickets for help on using tickets.