Skip to content

Tiled Galleries: Add new filter 'jetpack_gallery_attachments'#3249

Closed
webbistro wants to merge 2 commits intoAutomattic:masterfrom
webbistro:master
Closed

Tiled Galleries: Add new filter 'jetpack_gallery_attachments'#3249
webbistro wants to merge 2 commits intoAutomattic:masterfrom
webbistro:master

Conversation

@webbistro
Copy link
Copy Markdown

In short: let other plugins to override the query for attachments for Tiled Galleries gallery template.

See detailed explanation at https://wordpress.org/support/topic/tiled-galleries-4

@kraftbj kraftbj added Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Tiled Gallery A different way to display image galleries on your site, in different organizations and shapes. [Status] Needs Review This PR is ready for review. labels Jan 11, 2016
@kraftbj kraftbj changed the title Add new filter 'jetpack_gallery_attachments' to tiled-gallery.php Tiled Galleries: Add new filter 'jetpack_gallery_attachments' Jan 11, 2016
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a docblock. @webbistro - Could you add this when you get a chance?

 * Filters the XYZ.
 *
 * @module tiled-gallery
 *
 * @since 3.9.0
 *
 * @param (type) (description)
 */

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

@jeherve jeherve added this to the 3.9 milestone Jan 11, 2016
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somehow calling apply_filters here feels awkward to me. The get_attachments method disregards any incoming parameters, which is not how filters should work. What do you say we bring back the old code here and just filter the returned value in the get_attachments method? This way you will only need to change code in one place.

Thanks for contributing and please let me know if I can help with that.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I was a bit in hurry... I also need to reconsider the whole approach, since I am thinking about adding similar filters to WP core. I need a day or two. Thanks!

@zinigor zinigor added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Jan 12, 2016
@zinigor zinigor modified the milestones: 3.9.1, 3.9 Jan 18, 2016
@zinigor
Copy link
Copy Markdown
Contributor

zinigor commented Jan 18, 2016

I'm holding this PR until 3.9.1, please let us know if you have any plans to go forward with this PR.

@kraftbj kraftbj modified the milestones: 3.9.1, 3.9.2 Jan 21, 2016
@dereksmart dereksmart modified the milestones: 3.9.2, 3.9.3 Feb 24, 2016
@eliorivero eliorivero modified the milestones: 3.10, 3.9.3 Mar 4, 2016
@samhotchkiss
Copy link
Copy Markdown
Contributor

Closing until @webbistro updates with his rethought approach :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Tiled Gallery A different way to display image galleries on your site, in different organizations and shapes. [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. Touches WP.com Files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants