trendwerk / sphynx

A light & simple WordPress starter theme 😽
16 stars 2 forks source link

Galleries: Fancybox doesn't work on default settings #297

Closed sboerrigter closed 10 years ago

sboerrigter commented 10 years ago

Whenever a user inserts a gallery, the default images link setting is "link to attachment page". With this option the fancybox popup doesn't show up, because the images don't link to a file, but to a page with the file on it.

I think we should remove the "link to attachment page" option from the gallery options, or set the default option to "link to file". I have tried to achieve this with an action, but it doesn't work for galleries. I don't know how to solve this, so I asked a question about this on stack exchange: http://wordpress.stackexchange.com/questions/157191/set-default-link-type-to-file-for-image-galleries

I hope someone has a solution for this.

sboerrigter commented 10 years ago

Solved it which this code in misc.php

/**
 * Force galleries to link to image files, not to attachment pages
 */
function tp_link_galleries_to_files( $out ) {
    $out['link'] = 'file'; 
    return $out;
}
add_filter( 'shortcode_atts_gallery', 'tp_link_galleries_to_files' );
haroldangenent commented 10 years ago

I don't think this is good practice.

This would render it impossible to use a default WordPress function, without returning an error or otherwise notifying the user or developer what is happening. Sure, it gets the job done and it would be suitable as a solution for a specific website, but I don't think we should keep this code in our framework.

And personally (maybe another discussion), I would rather see misc.php disappear than add more stuff to it. I'm not really a fan of tweaking WordPress standards by default (in a framework), this file is something that could easily clutter.

So I'm reopening this for debate.

sboerrigter commented 10 years ago

Hmm.. okay. Maybe this isn't the best solution, but this is what I could do and I thought it did the trick. Do you have a suggestion on how we could do this otherwise?

haroldangenent commented 10 years ago

Well, this is probably more a JavaScript issue than something else: https://github.com/trendwerk/trendpress/blob/master/assets/js/functions.js#L6. Maybe we could fix that.

But maybe there are more elegant solutions out there, which don't require too much 'miscellaneous' code. I'd like us to research some alternatives, maybe even a replacement of Fancybox.

haroldangenent commented 10 years ago

This has been fixed in https://github.com/trendwerk/trendpress/commit/5a6ea8a6acbcd43b7900ef05bd64c6e563fe4842 and https://github.com/trendwerk/trendpress-child/commit/1be753deb2f15bd9476853cc950aa903dde1e587.

Fixed this by using JavaScript to force galleries to be treated as images, using the code below:

$( this ).find( 'a' ).fancybox( {
    type: 'image'
} );

Could you guys please review?

haroldangenent commented 10 years ago

Just committed a workaround for a bug in Fancybox itself: https://github.com/trendwerk/trendpress/commit/75daadda2fdfa301c84fb718bd72bf25f6adb63d & https://github.com/trendwerk/trendpress/commit/d6ae7a4a8f8f5f395a54d4c1d507d379f3b58e8e.

sboerrigter commented 10 years ago

Seems to work perfectly. Thanx!