AO3-7143 Adding filter to user collection page

[ReverM]

Issue

https://otwarchive.atlassian.net/browse/AO3-7143

Purpose

This adds the ability to filter collections on user pages like on the generic collections page.

Credit

Danaël / Rever (they / he)

[ReverM]

#5487 original PR. Reopened the PR since merging upstream made it quite angry. Apologies for the added complexity.

[ReverM]

@sarken pinging you since you commented in the previous PR

[ReverM]

It seems that the spec controller failure is not expected behaviour after this PR?

[sarken]

I checked on the failing spec, and it looks like it's failing because we lost alphabetical sorting on user collection pages. That's something we definitely want to keep; users weren't happy when we accidentally switched to created_at sorting.

[sarken]

We only put our subnavigation in div like this in rare instances where we have complex navigation with multiple separate sets of actions, like two lists, or a list and a form outside the list. The div on works/index.html.erb is there because we can have both a form and a list... but I think it's wrong, actually, because they shouldn't be there at the same time.

I'm pretty sure the reason you've added this is to make the navigation take up the full width of #main and push the filters into the right position on the page. Interestingly, the bookmarks index doesn't have this div, nor does it have that problem -- and after some investigating, I figured out why. Both bookmark and work index pages leave the empty ol.index on the page, while the collection index removes it with the unless @collections.blank? logic on line 52. Empty list elements aren't great for accessibility, so we don't want to replicate that here, either.

I think the solution we need here is to add a clear to form.filters. That should keep the filters in the right position without either the empty list or the div.

[sarken]

You can remove the role="navigation" while you're here -- it's not valid on list elements.

[sarken]

Instead of adding a class, let's take a look at what's causing the problem here. (But just a quick note for the future that we have some rules for adding classes.)

It looks like all collection index pages get the filtered class, regardless of whether they have filters, thanks to this:

otwarchive/app/helpers/application_helper.rb

Lines 60 to 62 in 7f6d5b1

	def page_has_filters?
	@facets.present? || (controller.action_name == 'index' && controller.controller_name == 'collections') || (controller.action_name == 'unassigned' && controller.controller_name == 'fandoms')
	end

otwarchive/app/helpers/application_helper.rb

Lines 19 to 25 in 7f6d5b1

	def classes_for_main
	class_names = controller.controller_name + '-' + controller.action_name
	show_sidebar = (!@hide_dashboard && (@user || @admin_posts || @collection || show_wrangling_dashboard))
	class_names += " dashboard" if show_sidebar
	class_names += " filtered" if page_has_filters?

Unfortunately, collections don't use @facets, so we can't just remove the collections logic from page_has_filters?. But we can tweak it to (controller.controller_name == "collections" && @sort_and_filter) or even just @sort_and_filter, since collections are the only place we use that variable, and I think it's reasonable to reserve it so it's only ever used to determine whether we display filters. #5721 also uses that variable for filtered challenge pages.

As a bonus, this approach should also fix the issue where /works/:id/collections pages

[ReverM]

I was trying to find the root cause but couldn't quite find it. Thanks!