MBS-12989 / MBS-12998: Edit list quick links improvements #2898
Conversation
|
@brainzbot, retest this please |
|
I tested the stray pipe problem and it's gone! |
There was a problem hiding this comment.
Much better code. Tested manually. Updated the already linked ticket MBS-12998 to cover hiding the link “Search for edits” too (as in the current second commit).
🚢
Related to this second commit (but that can be addressed in a later pull request): The paragraph “No edits were found matching your query. Please try again.” that is now displayed (and was never displayed at least since the React conversion) doesn’t seem to be useful. It says the same thing as the “Found 0 edits” which is already shown. The link named “try again” actually blanks search conditions, while it would expect the same search to be tried again. I would rather just remove this paragraph.
The actual results in subscribed_editors are already filtered to show only edits the editor hasn't voted on yet, but we forgot to add this restriction to refine_url_args (it's already there for other similar refinements).
This is supposed to hide the Search for edits link on ListHeader and show an extra message when accessing EditList from edit search, but we were never setting it.
This was kind of a mess with us trying to guess where the different | separators should go. Rather than doing this, I'm now just building an array of any appropriate quick links, then joining them with the separator as needed, as we do with FooterSwitch in ArtistIndex.
"Found 0 edits" is already shown, and the try again link is not helpful since it would drop the edit search parameters. The edit search form is still present on the page anyway to edit the search directly.
|
You're right about that no results message actually, it does more harm than good. Added a commit dropping it. |
There was a problem hiding this comment.
Super!
I wanted to do that but I didn't recognise the JavaScript I know.
In JavaScript I usually use join() but this .js file uses some high tech code I didn't understand, so I was clueless and could only repeat exact same existing code.
Fix MBS-12989 / MBS-12998
Problem
The quick links section has a few issues:
Solution
The first commit adds the missing predicate to the
refine_url_argsofsubscribed_editors.The second commit sets
isSearchwhen callingEditListfromsearch_results.tt, which I think was always supposed to happen - we literally didn't setisSearchanywhere at the moment.The third commit builds an array of any appropriate quick links, then joins them with the | separator as needed, as we do with
FooterSwitchinArtistIndex, rather than trying to guess when a separator will be needed. This also allowed me to group the conditions a bit better, which hopefully is clearer.Testing
Manually, navigating between all the different edit lists, when logged in and logged out.