WordPress.org

Make WordPress Core

Opened 9 months ago

Last modified 5 days ago

#52224 accepted enhancement

RSS Widget: allow removing the feed icon link

Reported by: sabernhardt Owned by: sabernhardt
Milestone: 5.9 Priority: normal
Severity: normal Version:
Component: Widgets Keywords: has-patch needs-refresh
Focuses: accessibility Cc:

Description (last modified by sabernhardt)

Removing the icon link can help fix at least these two bugs:

  1. #47670: Icon links for multiple widgets have identical link text.
  2. Hiding the icon without hiding/disabling the link creates an invisible link in themes such as Twenty Twenty-One (#52880).

On #47670, one possible solution was to remove the first link for everyone, but that would break probably all themes that float or hide the icon link with either .rsswidget:first-of-type or .rsswidget:first-child.

With a new filter, setting it to return false (or null) could solve both issues without breaking:
add_filter( 'rss_widget_feed_link', '__return_false' );
(Although, if a theme adds the filter and updates the styles accordingly, a child theme that replaces the parent stylesheet would likewise need updating to avoid any breakage.)

Another, separate possibility I've considered is including a new widget checkbox option so individual site owners can remove that link and/or include a link to the feed at the bottom of the widget instead. Then they can quickly add the link back if the layout breaks in their theme. If adding an option for the link within the heading, I'd like that to be unchecked by default, at least for any newly-added widgets. Ideally the heading would not contain both links.

Attachments (2)

52224.filter.diff (1.6 KB) - added by sabernhardt 9 months ago.
adding: filter, unique classes, internationalization, lazy loading
52224.2.diff (11.3 KB) - added by sabernhardt 3 weeks ago.
adding an icon display option to widget settings, updating unit tests, fixing lazy load attribute

Download all attachments as: .zip

Change History (12)

@sabernhardt
9 months ago

adding: filter, unique classes, internationalization, lazy loading

#1 @sabernhardt
9 months ago

  • Owner set to sabernhardt
  • Status changed from new to accepted

In addition to creating a filter for the icon link HTML, 52224.filter.diff:

  • Adds a unique class name to each of the links to help theme authors update styles with the :not() selector if necessary (example below is for Twenty Twenty)
    .widget_rss .widget-title a.rsswidget:first-of-type:not(.rss-widget-title) {
    	display: none;
    }
    
  • Adds internationalization function to translate "RSS" in alt text
  • Adds loading="lazy", as recommended on #50041
  • Moves the link-related variables inside the if($title) conditional

I expect this needs some polishing.

If desired, revisions to this filter could:

  1. Facilitate editing the heading text link as well (either adding an image inside or removing that link so the heading is only text).
  2. Encourage removing the icon link (within the docblock).
  3. Adjust the filter so we could consider a switch to opt-out later, if themes would not break by then.
Last edited 7 months ago by sabernhardt (previous) (diff)

#2 @sabernhardt
9 months ago

  • Keywords has-patch added

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


6 months ago

#4 @ryokuhi
6 months ago

  • Milestone changed from Awaiting Review to Future Release

Following feedback from the owner during the Accessibility Team's bugscrub today, moving to Future Release.

#5 @sabernhardt
6 months ago

  • Description modified (diff)

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


3 months ago

#7 @sabernhardt
3 months ago

  • Milestone changed from Future Release to 5.9

@sabernhardt
3 weeks ago

adding an icon display option to widget settings, updating unit tests, fixing lazy load attribute

#8 @sabernhardt
3 weeks ago

The RSS block does not include the icon (or the text heading), so this only affects the classic/legacy widget.

I added a checkbox setting for displaying the icon link in the latest patch. This should maintain the link (and automatically check the box) for any existing RSS widgets, but the checkbox should be unchecked when creating a new widget.

The patch now checks whether lazy loading is enabled before including the attribute. I also updated the unit tests' expected output.

#9 @audrasjb
11 days ago

That's a good patch idea :)
Shouldn't we show the icon by default to avoid introducing a breaking change?

#10 @sabernhardt
5 days ago

  • Keywords needs-refresh added

The main backward compatibility concern is making sure existing widgets keep the icon link, which the patch seems to do. However, most people who would create a new (legacy) RSS widget now probably would expect the link, so I'll switch the default checkbox value to checked. If we offer two methods to remove the icon, that is good enough.

Also, if a theme removes the link by setting the filter to return false, the checkbox option should not display in the widget's settings form.

Note: See TracTickets for help on using tickets.