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

Fix for feature requests #32 and #299 #6

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

rundgong
Copy link

With these fixes it solves both of my features from #299, and the duplicate from #32.
Query from URL populates the search box with input parameter query, if it exists, like this:
http://localhost:3000/search?query=search term

Searching for specific file types can now be done by prepending "-t XYZ" before the search term.
XYZ can be any of the allowed strings in the API, and also all the short hand alternatives that ncdc uses.

The parameter parsing is prepared for more parameters, but only -t does something at this point.

Specifying options in the search text itself is not as nice as having a dedicated drop down menu, but it should be good enough for the power users out there.

@maksis
Copy link
Member

maksis commented Dec 18, 2018

Would you be interested in implementing the dropdown menu for search types? This implementation would only be a temporary workaround as the search type selection should also be easily available for non-advanced users.

@rundgong
Copy link
Author

Yes, I think I can do it. I did some experiments, and it seems to work, but I'm not sure about the styling.

@maksis
Copy link
Member

maksis commented Dec 19, 2018

The file type selection dropdown should probably be a separate component, which fetches the search types from the API and shows the options using the generic dropdown component. There are also existing utils to get an icon for each search type.

I was thinking of a possibly collapsible (or popup?) advanced options panel, that could later also include other search options found from the Windows version (size limits, excluded words, hubs to search from and so on).

@rundgong
Copy link
Author

That is probably more than I can do. I don't know Node or React or even TypeScript really.
I have a static regular html drop down kind of working now, but that is probably as good as I can do, with the time I have available.

@rundgong
Copy link
Author

Now there is a drop-down also.

Do you know what the proper way to automatically execute the search would be?
When there is a url query it would be nice to also fire off the search, and not just populate the search field. I have some ideas but they feel like a hack. (And I don't even know if they work yet :-) )

@maksis
Copy link
Member

maksis commented Dec 20, 2018

You could do something similar that is currently being done in checkLocationState (just check location.search instead for the query changes). The current location should actually also be updated with the URL query when the user performs a search (and the search can then be sent to the API by detecting the URL change). searchString should actually be removed from the location state as the URL param can do the same job.

I'm going to be travelling during the next few weeks so I may not be able to respond to questions until I come back.

@rundgong
Copy link
Author

I don't fully understand everything going on in there, so I went with a "simpler" solution that I can understand. Just launch the search from render with a small delay and make sure it only gets called once. It does not "blend in" with the framework 100%, but is quite easy to understand.

@maksis
Copy link
Member

maksis commented Mar 14, 2019

Sorry for the delay with this pull request.

I've done some drafting with this feature as it should eventually be made available to a wider range of users.

image

It's just an early UI draft which can be viewed already when launching the UI in dev mode.

Copy link

sonarqubecloud bot commented Sep 6, 2024

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