Skip to content
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

Advanced Search Options (Attempt 2) #118

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

confused-Techie
Copy link
Member

This PR adds some advanced search functionality.

Mainly focusing on the ability to sort packages by the different supported criteria, as well as being able to set the direction of them.

I took the feedback from #77 and moved this behavior into the pagination bar, as well as adding everything to the dropdown.

Moving this elements outside of the actual form element used to make the request to the backend, did require some additional JavaScript to do, but also has allowed a better experience ensuring the shown defaults always match with whatever is currently on the page, as well as lets us keep the default values in the form, rather than set by classes on the elements themselves.

But as for some examples of how this functionality looks:

image

image

image

While the menu doesn't look revolutionary, it matches exactly with our existing menu used for theme selection.


I do plan to either add to this PR, or create another, which allows us to use the more fancy advanced search features, such as filtering by extension and such, but there's a couple of technical issues to figure out to do so.

@Daeraxa
Copy link
Member

Daeraxa commented Oct 1, 2023

So sorry this has taken so long for me to get to. I've been having trouble, at least in the dev version using the example yaml. It doesn't actually seem to do anything when you change the options. I assume this isn't an issue if using all the proper setup? Otherwise any idea why it refuses to change the order of anything?

I have a few points about UI, both minor in the sense they don't affect functionality, only how it looks.
The "Sort By" list displays vertically but the "Order" list displays horizontally. I think they should probably share a single UI element here seeing as they are related. This also gives the chance to set the width of the "Order" dropdown to the same width as the "Sort By" which means that clicking between them would give the effect of keeping the dropdown "container" static and only the content and the chevron move. At the moment the box expands and contracts in width (height would be less jarring).

I would submit a screenshot and PR showing what I mean but I'm finding the tailwind styles utterly impossible to comprehend and changes don't seem to be doing anything. From the tailwind docs I want to apply flex-col to the flexbox but it is utterly defeating me...

@confused-Techie
Copy link
Member Author

@Daeraxa One thing to note, this doesn't change any options live. We may be able to enable this behavior, but as of now, it just modifies the parameters that will be used the next time you execute a search. Meaning if you change an option you'll have to hit search again, otherwise this may cause loading everytime you change anything, and may become annoying. But if it's not clear, then we can see about making this happen.

As for the UI, I totally agree it's weird. Worst part is, we are using the exact same styling for both elements. The difference in vertical and horizontal is just Tailwind being Tailwind. But I'll try to see what I can do to make these more similar.

Otherwise I really appreciate you taking a look!

@Daeraxa
Copy link
Member

Daeraxa commented Oct 2, 2023

Aah I see, honestly I think that might be a problem. Once you know about it, sure, hitting search again makes sense but that isn't at all intuitive to me. If there isn't the possibility of changing the state of the live set of results, what about adding an onClick to the sort buttons to also make it search again?

Edit- re-read it your last message. To be honest I don't think the fact that "his may cause loading everytime you change anything" would be an issue. At most it means that the page will load twice but I genuinely think people are going to think it is broken.

Having the ordering at time of search is a feasible option but I think the UI has to reflect that by having the sorting as part of the search container itself (e.g. literally next to the search button) rather than on the pagination section. By having it there it implies to me that it should be acting on the displayed results like the pagination controls.

@confused-Techie
Copy link
Member Author

@Daeraxa You do make a really good point.

Having these buttons all down by the pagination settings makes it feel that it should be live.

And while it may be technically possible to sort these live client side, I'd rather not duplicate the logic on the frontend, when it already exists on the backend.

So in that case, I'll agree that it shouldn't be the biggest issue to live reload. Thanks a ton for this input, I'll get with these changes as soon as I can

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants