ScienceCommons / curate_science

Transparency & credibility curation products for all research stakeholders.
https://CurateScience.org
MIT License
13 stars 6 forks source link

Change the default text of the sort button on the recent page #124

Closed dolenda closed 4 years ago

dolenda commented 4 years ago

Update in the "minor esthetic/stylistic tweaks #96"

Recent page: Update text from "SORT BY: NEWEST FIRST" simply to "SORT BY" (just like on search results page, e.g. https://curatescience.org/app/search/?q=disgust) 5 points

eplebel commented 4 years ago

hmm.... are you sure this is ONLY removing the ": NEWEST FIRST" text, rather than nullifying the actual SORT criteria of the recent articles??

from my understanding of looking at the SortArticlesButton.jsx code, https://github.com/ScienceCommons/curate_science/blob/ccbe51599d17decf369d139f30cc5b13c1b96308/src/components/SortArticlesButton.jsx#L88 , it looks like all we'd need to do is remove the following piece of code { sorted_by ? <span>: { sorted_by }</span> : null } after the "Sort by" on line 88

dolenda commented 4 years ago

hmm i think it does the same thing as in the SearchResults page: it sets default "SORT BY" and null, which gives just "SORT BY", which is replaced by other labels "SORT BY" and ": NEWEST FIRST" or "SORT BY" and ": IMPACT" when one of them is clicked.

i assumed that we do not want to completely remove the second part (NEWEST or IMPACT) of the SORT BY label because a user can forget which criterion she/he used so she has to click it again to make sure.

or maybe not and we should simplify ui and remove the second part of the label completely as you showed, by removing. { sorted_by ? <span>: { sorted_by }</span> : null }. then the SORT BY label would be changed on the SearchResults page as well. after writing this and doing some more testing i guess that we can remove it completely :sweat_smile:

so you decide, just let me know

eplebel commented 4 years ago

ah OK, i did misunderstand parts of that. So yes your fix would be an improvement, but it would be better if you can update it so that even after an article list is sorted, it still only displays "SORT BY" (because on smaller screens, there's simply no horizontal room to display that much text, and then it wraps on a separate line and gets ugly, see screenshot below), for Recent page and Search results page.

Of course, the downside, as you mention, is that a user could forget the sort order, and must click on the Sort button to see the criterion, but that's still overall more optimal (though i guess a fancier/better solution would be to render different things based on the viewport size, but that's too much work given the priority level of this issue relative to all the other outstanding issues!)

image

dolenda commented 4 years ago

ok, i see now, thanks. i'm removing this { sorted_by ? <span>: { sorted_by }</span> : null }.

eplebel commented 4 years ago

ok thanks, and this will update it on both the Recent page and Search results page correct?

dolenda commented 4 years ago

yes, i rechecked it, it will update both, Recent and Search results pages