WordPress.org

Make WordPress Core

Opened 20 months ago

Last modified 11 months ago

#21569 new enhancement

Filter gallery styles with access to $attr

Reported by: betzster Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.5
Component: Media Keywords: has-patch
Focuses: Cc:

Description

Currently you can replace the entire output, but that's all or nothing — if you decide to do that, you also need to parse the attributes array, decide on good defaults, etc. You can also filter the styles, but you don't have access to the attributes array.

Use case: In my theme, I'd like to use the number of columns as more of a suggestion. I could set min-width to what $itemwidth is currently and it would be more responsive. If the page shrinks too small, it would only load 1 image per line instead of being locked into the right number of columns.

Attachments (2)

media.diff (1.8 KB) - added by drywallbmb 14 months ago.
patch referenced in comment 5
21569.diff (1.8 KB) - added by markoheijnen 14 months ago.

Download all attachments as: .zip

Change History (12)

comment:2 markoheijnen14 months ago

  • Milestone changed from Awaiting Review to 3.6
  • Version set to 3.5

I think we should do this for 3.6 too. The gallery shortcode is broken as it is. #15155 fixes it in a way. I also think that '<br style="clear: both" />' should be an argument. So people can easily clean it out or replace it with something else like a <hr>.

comment:3 drywallbmb14 months ago

I'd like to see two things happen here at the least. These are simple but could be handy...

  1. Add a gallery_output filter. This could/should be called around line 802 of wp-includes/media.php, right before returning $output.
  2. Fix line 299 of wp-includes to use double-quotes around the style attribute as in line 795. (or, alternatively, change line 795 to use single quotes). This is minor but the inconsistency is annoying.

I'd submit a patch but I haven't even done so before and it'll take me a while to remember how to use SVN, I've been on git for years... :)

Last edited 14 months ago by drywallbmb (previous) (diff)

comment:4 markoheijnen14 months ago

The gallery_output filter doesn't make sense to me. There is already the filter 'gallery_style'.

Your second point isn't inconsistency. That is how WordPress does it. I also don't like it. You are right if you meant the splitter (<br/>). One is with single and the other with double quotes.

drywallbmb14 months ago

patch referenced in comment 5

comment:5 drywallbmb14 months ago

  • Keywords has-patch added

The problem is that the gallery_style filter only runs on the string before it's actually been populated with items. Thus, you can't use to, for example, use a str_replace to get rid of all the <br> elements, because they haven't been tacked onto $output yet.

And yes, sorry for the ambiguity, I meant the splitter <br>'s — in the loop they're being output with double quotes in the HTML and after the loop the final <br> element is being output with single quotes around the style attribute.

For giggles (and/or clarity), I'm attaching my proposed patch, which normalizes all HTML element attribute values generated by gallery_shortcode() to be double-quoted, and adds the gallery_output filter making it possible to filter the entire gallery's markup. As mentioned, I'm rusty with SVN so apologies if the patch is borked.

comment:6 markoheijnen14 months ago

The patch isn't going to happen like this. Everywhere in WordPress they use single quotes for HTML. See enqueueing script and styles. So the splitter (<br/>) as an argument is the solution for this ticket.

The filter still doesn't make sense since the <br/> elements should be an argument. Doing a str_replace or a regex is wrong to do. Then you can better use the filter 'post_gallery'.

markoheijnen14 months ago

comment:7 markoheijnen14 months ago

I added a patch that creates a splitter argument and uses that. I choose for double quotes since it is better fitting here. I also added a additional class: gallery-link-{$attrlink?}. This gist show the reason why: https://gist.github.com/markoheijnen/4635255

@drywallbmb: About your patch. I also should have said that that kind of escaping isn't really what you will find in WordPress core code. It's then better too change from double quotes to single quotes. Unless it isn't posible what you can see in regex's.

comment:8 drywallbmb14 months ago

I'm with you 100% here. Using a $splitter argument eliminates the need for the filter I was proposing, and also nicely does away with the quotation mark inconsistency I was trying to eliminate. Nice work, let's all just pretend I never said anything. :)

Now what happens?

comment:9 DrewAPicture11 months ago

  • Keywords punt added

#15155 allowed us to filter shortcode attributes going forward with [23626].

I'd guess it's probably too late to add the $splitter arg and juggle up $link in time for 3.6.

comment:10 SergeyBiryukov11 months ago

  • Keywords punt removed
  • Milestone changed from 3.6 to Future Release
Note: See TracTickets for help on using tickets.