Skip to content

Conversation

harshil21
Copy link
Member

@harshil21 harshil21 commented Aug 31, 2022

Introduces Search-as-you-type and Advanced Search — the ability to search by the type of object — Class, Attribute, Method, Parameter, or just the documentation in general.

demo.mp4

Where can I test this?

https://docs.python-telegram-bot.org/en/doc-search/


Note for reviewers:

  • I made a fork of readthedocs/readthedocs-sphinx-search and used that here, since the number of changes to both the theme and the javascript was drastic enough. You can review the changes here. We can also discuss moving this to the PTB-organization if wanted.

Things which can be improved further:

  • Assign a keyboard shortcut for opening advanced search? Maybe also one to clear the input field.
  • The theme is dark mode by default. We can try to support light mode if we get enough requests. Implemented.
  • On mobile, the positioning of elements is currently broken. Fixed.
  • Design / colors of all elements! I tried to improve this as much as possible, but I'm no css wizard. Much improved! Suggestions on what to improve are welcome.

Things which can't (really) be improved:

  • Search results when there are typos in your query, or you're searching for two closely matching words. Since RTD's backend for search is ElasticSearch, one would have to follow the Advanced Search Syntax to get a better result.

@harshil21 harshil21 added enhancement ⚙️ documentation affected functionality: documentation labels Aug 31, 2022
@harshil21 harshil21 added this to the v20.0a5 milestone Aug 31, 2022
@harshil21 harshil21 changed the base branch from master to doc-fixes August 31, 2022 00:59
@Bibo-Joshi
Copy link
Member

Pfew, that's quite something that you've cooked up!
The search-as-you type is definitely nice. However, I'm not quite comfortable with the overall setup.

I feel like the things like toggling the result types, adjusting the theme (colors and light/dark mode) and mobile support are things that go beyond "I'm doing a little tweaking" and should rather be proposed to upstream. Maintaining this exclusively for/as part of PTB is not something that I would look forward to.
Recall that we switched from the rtd theme to furo exactly b/c we were starting to fiddle with small details in the theme :D

@harshil21
Copy link
Member Author

Yes, I did think about writing an issue for this but then I was not sure what I would ask them since:

  1. The official RTD theme doesn't actually support dark mode, let alone them modifying their design for e.g. Furo's built in dark mode. Switching on darkreader just fixes it for both cases
  2. I could ask for advanced search, but that still wouldn't support parameters, since that relies on sphinx-paramlinks.
  3. the upstream version does support mobile properly, it's just my modding has changed the position of the elements (you have to scroll right to access the dropdown menu). I'll try to fix this, should be simple

So I decided to just fork and make own changes. Maintaining shouldn't be too much of a problem? Worst case would be that it would just fallback to regular default search. I do think here that the positives outweigh the negatives

@Bibo-Joshi
Copy link
Member

I see your points, but have a different view point regarding the negatives & the positives … at least until support for mobile devices is enhanced (apart from the "search for" toggle being way off to the right, the search box is "striked through" by the blue separator bar, at least for me)

@Bibo-Joshi
Copy link
Member

LGTM :) @Poolitzer do you want to have a look as well?

@Poolitzer
Copy link
Member

Just from a quick look at it, the type button looks misaligned. And it doesn't say no results if I filter out existing results for my query.

image

@Poolitzer
Copy link
Member

Yes I like the CSS look now good job! I just realized going backwards/forwards one page does not show the search again, is that fixable? Even worse if you filter stuff out, all of that apparently generates a new page. Makes you think the browser broke :D

Copy link
Member

@Bibo-Joshi Bibo-Joshi left a comment

Choose a reason for hiding this comment

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

I guess from my side everything's fine now, just two tiny nitpicks below. @Poolitzer do you have any change requests left?

@Poolitzer
Copy link
Member

Almost happy. Back and forth works great now, but if you

  1. type something in the search bar
  2. press enter so the "old" search site is opened
  3. now press the search bar on the left again

The search is not triggered. Is that fixable? Then I am happy.

@harshil21 harshil21 merged commit 1491f5e into doc-fixes Sep 22, 2022
@harshil21 harshil21 deleted the doc-search branch September 22, 2022 13:59
@github-actions github-actions bot locked and limited conversation to collaborators Sep 29, 2022
@Bibo-Joshi Bibo-Joshi added 🔌 enhancement pr description: enhancement and removed enhancement labels Nov 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
⚙️ documentation affected functionality: documentation 🔌 enhancement pr description: enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants