Skip to content
This repository has been archived by the owner on Apr 18, 2024. It is now read-only.

Add ability to pre-fill the search bar from URL param #30

Merged
merged 4 commits into from
Apr 15, 2017

Conversation

icyflame
Copy link
Member

@icyflame icyflame commented Apr 7, 2017

This will enable the addition of MFQP as a custom search engine in Chrome. (with a URL structure something like: https://qp.metakgp.org/?search=%s

I have tested this change, and it works well.

Copy link
Member

@athityakumar athityakumar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A minor change required.

Irrespective of the query in the url param, a / gets added at last - which messes up the search results at times.

Search as URL param :
image

Search as input to search bar :
image

Notice the 2nd result missing in case 1. Maybe just the / can be removed from the search_query variable in line 107?

@athityakumar
Copy link
Member

@icyflame - I have sent a PR for the same. Have a look! 😄

@icyflame
Copy link
Member Author

icyflame commented Apr 7, 2017

image

I added that search engine to Chrome 56. Then, I searched using the address bar. There was no / that was appended.

Irrespective of the query in the url param, a / gets added at last - which messes up the search results at times.

Who adds the /? If the user manually adds it, that's something that they have chosen to. I don't see the browser adding anything by default even when I manually type out the URL.

@athityakumar
Copy link
Member

It gets added by the browser itself (I think. Can't think of any other reason), after the user has typed his search param and pressed the Enter button. For example,

Search param :
image

Result URL shows a / at the end :
image

I've added it as a search engine to Google Chrome Version 56.0.2924.87 (64-bit).

@icyflame
Copy link
Member Author

icyflame commented Apr 7, 2017

@athityakumar Can you post the search engine entry here? You might have put a / after the %s. If you add the search engine URL as https://localhost:8000/?search=%s the / won't be appended.

In any case, it will be a good idea to remove trailing forward slashes. So, you should change your PR on the other issue to ensure that the last character is a forward slash before slicing it away. 😄

@athityakumar
Copy link
Member

image

Sure, I'll do the required changes on that PR. 😄

@athityakumar
Copy link
Member

@icyflame - Um, isn't it the https://localhost:8000/?search=%s?

@icyflame
Copy link
Member Author

icyflame commented Apr 7, 2017

Hmm, okay, I guess your browser decided to append the / to the query. Mine doesn't. Weirdly enough, I am on the exact same build of Chrome: Version 56.0.2924.87 (64-bit)

Okay, I will merge that PR and that should update the commit here.

@athityakumar
Copy link
Member

Weird o weird. 😕

I've updated the other PR. 😄

Removes / at end of search url param
@icyflame
Copy link
Member Author

icyflame commented Apr 8, 2017

@athityakumar Merged. I have tested with multiple trailing forward slashes, works fine.

@athityakumar
Copy link
Member

@icyflame - No more issues from my side, I've approved the PR. Let's wait for reviews of @vivekiitkgp & @amrav before merging this in. 😄

@icyflame
Copy link
Member Author

@vivekiitkgp and @amrav Please review this PR. 🙂

I will merge this PR in a few hours. Thanks!

@icyflame icyflame merged commit e2cad9a into metakgp:gh-pages Apr 15, 2017
@icyflame icyflame deleted the add-search-url-param branch July 18, 2017 10:51
@icyflame icyflame mentioned this pull request Dec 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants