Display jump links above combined search results

[maccabeelevine]

The idea here is to optionally provide a sort of table-of-contents above combined search results, each item a jump link within the page to the bento box below.

Inspiration examples from USC:

and Cornell discovery layers:

[maccabeelevine]

Here is a first take on the idea for boostrap3. Some styling advice would be welcome. It probably needs at least some surrounding box similar to the facet and search result summaries above.

[maccabeelevine]

Updated screenshot, I added record counts to each jump link, similar to one of the examples.

[demiankatz]

I'm short on time so haven't given this a very deep review yet, but I did notice one thing that might be easy to improve.

Also, in line with my comments on an earlier PR, have you tested that this works with AJAX-loaded content? I see you do have some Javascript here, but it wasn't clear to me at a glance if that would handle the AJAX situation or if it was for a different purpose.

I'd also be interested to hear @crhallberg's thoughts on the aesthetic side of things. I might have opinions of my own when I have time to try this hands-on as well, but that may be a little while, and I thought I might as well share my initial thoughts in the meantime.

[maccabeelevine]

Also, in line with my comments on an earlier PR, have you tested that this works with AJAX-loaded content? I see you do have some Javascript here, but it wasn't clear to me at a glance if that would handle the AJAX situation or if it was for a different purpose.

@demiankatz If I understand the question, yes it does work with both ajax=true and ajax=false modules defined in combined.ini (and a mix of both).

I'd also be interested to hear @crhallberg's thoughts on the aesthetic side of things. I might have opinions of my own when I have time to try this hands-on as well, but that may be a little while, and I thought I might as well share my initial thoughts in the meantime.

Yes please @crhallberg and others. I didn't spend any time really on aesthetics yet because I don't know how "far" to go with it. I.e. I think the two non-VuFind examples I provided look great, but neither would be consistent with bootstrap3's general design. Suggestions please.

[maccabeelevine]

I noticed yesterday this doesn't work right for sections with a filter. Working on that this morning.

[maccabeelevine]

I noticed yesterday this doesn't work right for sections with a filter. Working on that this morning.

Fixed.

[demiankatz]

Thanks, @maccabeelevine -- I finally had time to look more closely at this, and I spotted one significant bug and one possible optional improvement.

Additionally, I notice that the jump links have no left margin, at least in the bootprint3 theme; that might be worth adjusting too:

Since this adds a new language string, we're really brushing up against the translation file freeze for release 9.1. If it's important to you to get this included in that release, maybe we can merge the new language string independently of the rest of the code so that it can be included in the translation process while giving us more time to work through the remaining issues here. What do you think?

[maccabeelevine]

Since this adds a new language string, we're really brushing up against the translation file freeze for release 9.1. If it's important to you to get this included in that release, maybe we can merge the new language string independently of the rest of the code so that it can be included in the translation process while giving us more time to work through the remaining issues here. What do you think?

Good idea. I've pulled out the translation as #3119.

[maccabeelevine]

Additionally, I notice that the jump links have no left margin, at least in the bootprint3 theme; that might be worth adjusting too

@demiankatz Yes that's true, I hadn't really tried to make it look good initially:

I didn't spend any time really on aesthetics yet because I don't know how "far" to go with it. I.e. I think the two non-VuFind examples I provided look great, but neither would be consistent with bootstrap3's general design. Suggestions please.

But I should have at least done something clean to start. I've given it some side margins. I also stopped it from wrapping within a single jump link. Current look:

[demiankatz]

Looks good to me! As with #3077, I recommend leaving this open another couple of days in case @crhallberg has anything to add before leaving on vacation. :-)

[crhallberg]

There's an AJAX bug holding this PR back, where we need to add this to the viewParams on line 125 of the CombinedController.

'domId' => 'combined_' . str_replace(':', '____', $sectionId),

[demiankatz]

Thanks for catching that, @crhallberg -- I thought @maccabeelevine and I had both tested the scenario that caused this bug (an ajax-loaded filtered column), but it appears that we missed that edge. I've confirmed that merging your fix corrects the underlying problem (which was causing the count of the filtered column to appear next to the unfiltered column instead of its proper place), so I'm going to dismiss your review as completed and merge this now.

Problem has been addressed.

[maccabeelevine]

Thanks @crhallberg for catching that and the fix!