After #1804614: [meta] Consolidate theme functions and properly use theme suggestions in core lands, this issue should be changed to active.

We would like to eliminate theme_tablesort_indicator in favor of theme_image but we should add a theme hook succestion so that the table sort indicator can be easily overridden.

Comments

LewisNyman’s picture

I'd like to expand the scope of this issue to discuss whether we need the table sort indicator to be an inline image. It's clearly presentational.

thedavidmeister’s picture

alternative potentially being something like remove the img tag completely + add the up/down arrow icon back in with CSS on the table header by doing something in tablesort_header() and add back in an equivalent solution for the title/alt tags explaining the sort behaviour that are currently on the image for accessibility reasons, also in tablesort_header().

jenlampton’s picture

Issue tags: +Twig, +Template consolidation

tagging

joelpittet’s picture

@LewisNyman and @thedavidmeister one counter reason to that would be CSS images don't like to to load in parallel, AFAIK so it would be a bit slower, and also someone would likely like to replace those images with maybe an icon webfont or svg?

Though if we were to go in the CSS direction I'd hope for maybe spriteing some of these?

joelpittet’s picture

I'm +1 for getting this in CSS:)

Cottser’s picture

mgifford’s picture

Status: Postponed » Active

What is this postponed on? At this stage it should be bumped up to 8.1.0 or addressed shouldn't it?

joelpittet’s picture

Issue tags: +CSS, +frontend

Good question, maybe just that all consolidation was postponed...

Tagging with CSS as I hope for that solution:)

LewisNyman’s picture

Title: Remove theme_tablesort_indicator, use theme_image instead, with appropriate theme hook suggestion » Remove theme_tablesort_indicator, use CSS instead
mitokens’s picture

sqndr’s picture

Assigned: Unassigned » sqndr

I will look into this issue this week.

sqndr’s picture

Status: Active » Fixed

The theme() function has already been removed.

Cottser’s picture

Title: Remove theme_tablesort_indicator, use CSS instead » Remove tablesort-indicator.html.twig, use CSS instead
Assigned: sqndr » Unassigned
Status: Fixed » Active

I think we still want to remove it even though it's a Twig template now :)

joelpittet’s picture

Oh yes, thanks! Yeah this doesn't need to be a template and even if it was a twig template I'd be tempted to put it in the table.html.twig to avoid the performance hit.

CSS approach would be simpler though.

lokapujya’s picture

interesting question: use a template or remove it? wondering what the reason is for removing it.

Cottser’s picture

Because it's a tiny template, and CSS is a thing.

joelpittet’s picture

Version: 8.0.x-dev » 8.1.x-dev

Bumping to D8.1

andypost’s picture

Version: 8.1.x-dev » 9.x-dev
Status: Active » Postponed

Stable theme has this template so probably this becomes 9.x

catch’s picture

Version: 9.x-dev » 8.2.x-dev
Status: Postponed » Active

No we can stop using the template in core in a minor release, just can't completely remove it.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.