Force recommended results to the top when sorting from a Category page #8103
Conversation
Codecov Report
@@ Coverage Diff @@
## master mozilla/addons-frontend#8103 +/- ##
=========================================
+ Coverage 98.09% 98.1% +<.01%
=========================================
Files 260 260
Lines 7363 7370 +7
Branches 1331 1333 +2
=========================================
+ Hits 7223 7230 +7
Misses 126 126
Partials 14 14
Continue to review full report at Codecov.
|
| @@ -30,6 +31,7 @@ import Select from 'ui/components/Select'; | |||
| import './styles.scss'; | |||
|
|
|||
| const NO_FILTER = ''; | |||
| const sortSelectName = 'sort'; | |||
There was a problem hiding this comment.
Could you use paramsToFilter.sort from src/core/searchUtils.js instead?
There was a problem hiding this comment.
Good idea. I think I'll keep sortSelectName but set it to paramsToFilter.sort. I'm not sure if you meant to get rid of it entirely and just use paramsToFilter.sort in the code, but I think having the name makes it clearer that there's a relationship between the two uses.
There was a problem hiding this comment.
It's fine to keep sortSelectName :)
| expect(root.find('.SearchFilters-Sort')).toHaveProp('value', sort); | ||
| }); | ||
|
|
||
| it('selects the second sort criterion in the sort select when there are two criteria', () => { |
There was a problem hiding this comment.
Shouldn't we try to select the criterion that is not recommended instead or should we have a multiple select?
There was a problem hiding this comment.
I like the idea of making the the first criterion that is not recommended, so I've implemented that change. I don't think we want to complicate it by allowing the user to select multiple sort criteria (in which case they'd also have to be able to specify which is the first sort and which is the second), but we can open that up for debate and a follow-up could be filed for that.
@jvillalobos Do you think we should enhance the sort form to allow users to specify multiple sort fields (now that we're sometimes forcing the recommended sort as the first criterion)?
There was a problem hiding this comment.
No, I don't think that will be necessary. Prioritizing recommended extensions should be invisible to the user.
There was a problem hiding this comment.
No, I don't think that will be necessary. Prioritizing recommended extensions should be invisible to the user.
Well, it won't be invisible if they look at the URL, but it won't be evident from the form. I assume that is acceptable?
bobsilverberg
force-pushed
the
category-search-sort-8084
branch
from
c59a692
to
0c5bf3a
Compare
Fixes mozilla/addons#13221