Twenty Thirteen: Add image navigation to attachment pages

[obenland]

Due to the standard header image we should add an image navigation enhancement similar to what Twenty Eleven had (add an anchor hash to navigation links). Currently it can be pretty annoying to navigate through a gallery on attachment pages with a screen that is less than 900px high.

[lancewillett]

+1, nice usability enhancement.

[DrewAPicture]

23543.diff​ appends the #main anchor to the attachment links.

I chose #main because it's the latest-running div where the next/previous links aren't hidden below the fixed navbar.

And, because it took me several minutes to work my way through the chain, I'll explain the filter.

1. next/previous_image_link() calls adjacent_image_link().
2. adjacent_image_link() retrieves the formatted link via wp_get_attachment_link().
3. wp_get_attachment_link() calls get_attachment_link() to retrieve the actual URL.
4. We filter the attachment link URL via the attachment_link filter in get_attachment_link().

[obenland]

Nice! I wonder if it would make sense to add ​the checks that Toolbox has going. What do you think?

[DrewAPicture]

Replying to obenland:

Nice! I wonder if it would make sense to add ​the checks that Toolbox has going. What do you think?

That's an interesting take on it.

I don't think we to do the extra checks though because adjacent_image_link() only pulls attachments with the mime-type of 'image' anyway.

[obenland]

23543.1.diff​​ is based off of DrewAPictures work: Moved the function before customizer code to maintain code flow, adjusted comments to Twenty Thirteen style and added a check for images only. We don't really need the anchor on other attachment pages.

The other checks in Toolbox were there to prevent 404s on unattached images when not using pretty permalinks. I was unable to reproduce that in trunk.

[kwight]

Patch works for me. #main makes sense for the anchor, but the title gets cut off by the fixed navbar.

[lancewillett]

In 23523:

Twenty Thirteen: usability improvement for better viewing of images on attachment pages. Props DrewAPicture and obenland, fixes #23543.

[lancewillett]

Note: I wasn't able to repeat the cut-off navbar.

[kwight]

Ah, it has to do with the admin bar (working from r23533).

When logged in, #main slides up behind both the admin bar and the fixed navbar, resulting in the title and post meta being covered up (except for an ugly artifact on the right from bottom of the meta).

When not logged in, #main only slides up behind the fixed navbar, leaving the title and meta showing but cut off.

I admit neither looks good to me. For fun I even tried the higher up #navbar as the anchor, but that just caused page jumping because of the fixed navbar.

Possible solutions? Maybe a JS hack to add top margin when the bars are present (ick). Of course, with no fixed navbar, everything is perfect :)

[DrewAPicture]

Replying to kwight:

When not logged in, #main only slides up behind the fixed navbar, leaving the title and meta showing but cut off.

I admit neither looks good to me. For fun I even tried the higher up #navbar as the anchor, but that just caused page jumping because of the fixed navbar.

I suppose we could insert a special jump point where we needed it, but that feels like a band-aid.

[lancewillett]

See also #23719, a bug that surfaced and results in this improvement being reverted.

[lancewillett]

In 23677:

Twenty Thirteen: revert addition of twentythirteen_attachment_link() in r23523 -- as discussed in the following ticket comments, the tradeoff means simpler code instead of a minor UX enhancement. Closes #23543 and #23719.