-
Notifications
You must be signed in to change notification settings - Fork 76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sort Library Components #1038
Comments
@bradenmacdonald As I see it, this functionality is going to be added in |
Sure, that makes sense. Though we may implement it for all tabs at once, rather than implementing each tab separately. After all the tabs are very similar. |
Hi @jmakowski1123 CC @bradenmacdonald
Should selecting this option also filter out any components which have not been published? |
Hi @jmakowski1123 CC @bradenmacdonald Since we're also doing keyword search, I kept the default sorting as keyword search relevance. To make this clear to users (and make my code simpler), I chose to show this option to users at the top of the list of sort options:
|
Correct, the Recently Published filter should remove any content that is not yet published. |
I think it seems redundant to indicate to users that the default sorting is by most relevant - I think that will be assumed. And if a user wants to override that, they can choose which sort they want, which I think is also intuitive. I would suggest to not include the "Most relevant" as a sort option, and rather assume that users will understand that this is the default behavior. |
EDIT -- Nevermind, I rebased and found the "clear filters" action, so can use that instead of showing a menu option. |
@jmakowski1123 If there are no components published, then we see the "You have not added any content to this library" message when sort/filtering by "Recently Published". If filters are applied, we can show another message and/or link to the "clear filters" action. How should that message read? |
@pomegranited I think it would be clearer to users if the messages reflect the specifics of the sort status. So in the case of having a filter applied for "Recently Published" but no components published, then the message could say "You have not added any recently published content." |
@jmakowski1123 Ach, there's another edge case -- if search keywords and/or tag or block type filters are applied + sorting by "recently published", then we'd need a more complex message.. So I chose to do this instead: Is that clear enough? |
Hi @jmakowski1123 -- I've deployed these changes to the sandbox so you can see them in action: https://app.tagging-preview.staging.do.opencraft.hosting/course-authoring/libraries Couple of things to note:
|
FYI this bug has been fixed and deployed to the sandbox. I created
|
Let's remove a sort filter by clicking on the current "sort by" selection. For example, clicking on Sort > "Title, A,Z" will sort by that, clicking it again will remove that Sort and take the user back to default. @jmakowski1123 any thoughts? |
That's a great idea, and easy to implement. Should "clear filters" continue to reset Sorting to the default (sort by relevance)? What do you think, @jmakowski1123 @sdaitzman @marcotuts? |
Hi @pomegranited, we discussed this some more with Jenna and decided to clarify the sort dropdown's design. Here's the updated version from Figma (rolling it out across the rest of the designs soon): Since the content is/should be always sorted somehow, we'd like to show the current sort in the dropdown instead of allowing users to "clear sort." The "Sort By" title within the We're thinking of filter / sort as separate concepts in this interface, and Recently Modified will be the default sort to surface blocks with recent changes. |
Thanks for this updated design @sdaitzman ! To be clear, are you asking for the following changes to the components sort UI?
My only concern here is that this means we lose the current (implicit) default sort option, which is to sort by "relevance" using the keyword(s) entered. Could "Relevance" or "Keyword Relevance" be added to this list somewhere, even if it is not the default option? |
@pomegranited - Curious for your thoughts from a feasibility standpoint. From a product pov, I see two options here:
Is one more feasible or much less effort than the other? Or do you see other options that I'm not seeing? |
@pomegranited thanks for confirming the UI details; some of those changes were intentional, and some came from Paragon component defaults. Sorry for creating any confusion with that mockup. I hope this helps clarify the UI details:
When there is an active keyword search, if we have a "most relevant" heuristic (e.g. weighting by keyword match / fuzzy search distance / matched fields) I think we should default to that option, and show it within the sort dropdown. This is partly a product/scope question that depends on which search heuristics exist, so I'll defer to @jmakowski1123 and those two options |
Thank you for those clarifications, @sdaitzman 💯
I think it may be confusing to the user to have the default sort changing out from under them. It also adds complexity to the code, but if that's what is needed, we can do it. Happy to defer to your expertise. |
@jmakowski1123 Oh sorry, I missed your comment before I replied above.
I think this will be the most straightforward option to implement. I'll give this approach a try and put it up a sandbox for testing. |
Hi @jmakowski1123 @sdaitzman @lizc577 @marcotuts , there's a partially-working sandbox up that demonstrates the sort widget changes requested here. See #1222 for details and screenshots. Unfortunately the sandbox isn't letting me create library components, but you can at least see the updated default sort options, and how they change when you enter a keyword into the search box. 🎓 LMS
If this looks ok enough, I'll get #1222 merged and deployed to the tagging-preview sandbox where it can be tested for reals. |
Thanks @pomegranited. One minor nitpick is that in its |
@sdaitzman Thank you for your review!
Interesting.. I'm seeing that issue on the sandbox, but not in my dev environment, so I think that issue is an artifact of my half-built sandbox. Also, we're not modifying the styles provided by Paragon here at all, so there's no custom trickery to fix. So I'd like to proceed with merging here, and if this crops up again on the real sandbox, I'll look deeper. |
Hi @pomegranited, that sounds good to me. Thank you! |
@jmakowski1123 @lizc577 @sdaitzman @marcotuts This is ready for AC testing on the sandbox, including the latest changes. See lib:OpenCraft:sortLib which contains these components:
|
This looks good to me! |
✅ In Scope:
a. Title, A-Z
b. Title, Z-A
c. newest
d. oldest
e. recently published
f. recently modified
?sort=created
but should not create a new history entry, so pressing BACK in the browser goes back to the previous page, not the previous sort option.The text was updated successfully, but these errors were encountered: