classification
Title: move quick search box above TOC
Type: enhancement Stage: resolved
Components: Documentation Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: berker.peksag Nosy List: Jelle Zijlstra, ammar2, berker.peksag, docs@python, eric.araujo, ezio.melotti, georg.brandl, python-dev, r.david.murray, zach.ware
Priority: normal Keywords: easy, patch

Created on 2013-11-04 06:28 by georg.brandl, last changed 2016-09-09 20:16 by zach.ware. This issue is now closed.

Files
File name Uploaded Description Edit
searchbar.diff ammar2, 2016-07-06 23:28 review
searchbar.diff2 ammar2, 2016-07-24 01:28 review
searchbar_in_header.diff ammar2, 2016-08-03 20:19
searchbar_in_header.diff2 ammar2, 2016-08-03 20:39
searchbar_in_header.diff3 ammar2, 2016-08-10 03:18
searchbar_in_header.diff4 ammar2, 2016-09-09 20:05
Messages (21)
msg202091 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2013-11-04 06:28
From Mark Summerfield:,

    The Quick search box is really useful, but it is annoying that its
    position varies from page to page. On pages with long tables of
    contents it is often out of sight.

    Why not put it above the Table of Contents. That way it is always
    in the same place and easily accessible.
msg228278 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-10-02 23:01
If this is referring to the box on docs.python.org shouldn't this be logged elsewhere?
msg267460 - (view) Author: Jelle Zijlstra (Jelle Zijlstra) * (Python triager) Date: 2016-06-05 19:02
Yes, I don't think we control this, the layout is generated by sphinx.
msg269913 - (view) Author: Ammar Askar (ammar2) * (Python triager) Date: 2016-07-06 23:28
It looks like this can be fixed by us. Since sphinx 1.0 there is this handy config parameter: http://www.sphinx-doc.org/en/stable/config.html#confval-html_sidebars

I've attached a patch which pins the searchbox as the first thing on every page.
msg270843 - (view) Author: Ammar Askar (ammar2) * (Python triager) Date: 2016-07-19 21:12
bump, added the documentation experts list, not sure if the core dev who made this issue is still active.
msg271106 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2016-07-23 19:52
The patch needs some work, as it stands it makes the 'Report a Bug' link disappear.  I haven't looked into it far enough to determine why.

For an alternative bikeshed color, would it be possible to put the quick search in the header, just left of the 'previous | next | modules | index' links?
msg271119 - (view) Author: Ammar Askar (ammar2) * (Python triager) Date: 2016-07-24 01:28
Thank you for pointing that out to me, it completely slipped past.

It looks like the 'Report a Bug' link disappears because deprecated api is used in Doc/tools/templates/layout.html

{% block sidebarsourcelink %}
{% endblock %}

http://www.sphinx-doc.org/en/stable/templating.html#blocks

>The following four blocks are only used for pages that do not have
>assigned a list of custom sidebars in the html_sidebars config
>value"

I've amended the patch to fix this by using the way they recommend with a custom html_sidebar.


As far as putting it in the title bar goes, I think it's slightly more prone to breakage since we're essentially duplicating sphinx's searchbar code. It's easy enough to add it to the right of those links, there's a {% block relbaritems %} for that. Adding it to the left is slightly more complicated. Personally I like just pinning it to the top of the sidebar but I can look into that as well if you really want. 
https://github.com/sphinx-doc/sphinx/blob/master/sphinx/themes/basic/layout.html#L27-L46
msg271918 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2016-08-03 18:05
This LGTM.

If it's simple enough, I'd like to see a shot at putting the search box in the top bar, but it's not necessary :)

I'll commit this in the near future unless somebody beats me to it.
msg271925 - (view) Author: Ammar Askar (ammar2) * (Python triager) Date: 2016-08-03 20:19
This patch adds the search bar to the navigation/header area on the right.
msg271927 - (view) Author: Ammar Askar (ammar2) * (Python triager) Date: 2016-08-03 20:39
Amended the navigation bar patch to add back a change that only shows the searchbar if javascript is enabled.
msg271929 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2016-08-03 20:51
I really like the search box in the header/footer.

I can't really speak for the actual content of the patch since my Sphinx/HTML/CSS/Jinja/whatever else knowledge is severely limited, but it looks pretty straightforward and the result is very nice.  I'd appreciate a review from someone who's better-versed in all things Sphinx (Georg, if you have a chance, you'd be ideal).

If there are no objections within about a week, though, I'll just go ahead and commit it.
msg272279 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-08-09 23:34
This looks pretty good to me, thanks! I have two minor suggestions:

1. It would be better to make "Quick search" a placeholder:

   <input type="text" name="q" placeholder="{{ _('Quick search') }}" />

2. <li class="right"> will add an unnecessary border in search page. I'd change it to

   <li{%- if pagename != "search" and builder != "singlehtml" %} class="right"{% endif %}>

Also,

    +form.inline-search input {
    +    display: inline;
    +}

may cause weird compatibility problems in some (old?) browsers. We can change it to use display: inline-block instead.

Is there a way to send this to upstream Sphinx?
msg272296 - (view) Author: Ammar Askar (ammar2) * (Python triager) Date: 2016-08-10 03:18
> 1. It would be better to make "Quick search" a placeholder

Do you mean in addition to the "Quick search" text that is already on the page or do you want to remove that? Because placeholder isn't fully compatible with some older browsers http://caniuse.com/#feat=input-placeholder

> 2. <li class="right"> will add an unnecessary border in search page. I'd change it to

Good idea, will fix this.

> may cause weird compatibility problems in some (old?) browsers.

What problems are you thinking of? I think `display: inline` is the right choice here since we want the inputs to inherit the height of the header. The page renders fine on all versions of IE from my testing.

Also, newer versions of sphinx come with a newer jquery that breaks compatibility with IE8 which would cause the search box to not appear anyway.

>Is there a way to send this to upstream Sphinx?

I don't think so, it's one of those things that make sense as a theme, which is perfect since we have our own sphinx theme anyway.
msg273574 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-08-24 15:55
Moving back to patch review for someone to sign off on the changes.
msg274286 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2016-09-03 04:38
Berker, would you mind giving this another look?  If you give it the OK, I can get it committed if you don't beat me to it.
msg274292 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-09-03 12:33
> Do you mean in addition to the "Quick search" text that is already on the page or do you want to remove that?

The latter. All of the popular browsers (Chrome, Firefox, Edge and even Safari) already support the placeholder attribute so I think we can safely ignore IE 8 :)

> What problems are you thinking of?

Honestly, I can't remember now, but I was talking about old Chrome and Firefox versions. Let's ignore my comment for now.

I have two more minor comments:

1. +    <div class="inline-search" style="display: none" role="search">

   I know this is basically a copy of upstream searchbox.html, but we can probably remove ``style="display: none"`` and ``$('.inline-search').show(0);``.

2. +    <li class="right inline-search">

   inline-search doesn't seem to be necessary here. ``display: inline`` is already applied by ``div.related li``.

I also found a regression in search.html. It doesn't show or highlight the search
words provided in the URL. Try the following URL to reproduce the problem: https://docs.python.org/dev/search.html?q=append

Screenshot from docs.python.org: https://dl.dropboxusercontent.com/u/166024/highlight-docs.pyo.png

Screenshor from my local copy with searchbar_in_header.diff3 applied: https://dl.dropboxusercontent.com/u/166024/highlight-after.png
msg274307 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-09-03 16:14
> I also found a regression in search.html.

Please ignore this. It doesn't have anything to do with your patch.
msg275415 - (view) Author: Ammar Askar (ammar2) * (Python triager) Date: 2016-09-09 20:05
>The latter. All of the popular browsers (Chrome, Firefox, Edge and even Safari) already support the placeholder attribute so I think we can safely ignore IE 8 :)

Roger, I've changed it to placeholder.


>I know this is basically a copy of upstream searchbox.html, but we can probably remove ``style="display: none"`` and ``$('.inline-search').show(0);``.

The reason that exists is because sphinx's search relies on javascript.


>inline-search doesn't seem to be necessary here. ``display: inline`` is already applied by ``div.related li``.

Good catch, I've removed the class from there.
msg275418 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-09-09 20:12
New changeset 274b25cd501f by Zachary Ware in branch '3.5':
Issue #19489: Move the search box from sidebar to header and footer.
https://hg.python.org/cpython/rev/274b25cd501f

New changeset 0f94a8fa5445 by Zachary Ware in branch 'default':
Closes #19489: Merge with 3.5
https://hg.python.org/cpython/rev/0f94a8fa5445
msg275419 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-09-09 20:16
New changeset 8927417c5e88 by Zachary Ware in branch '3.5':
Issue #19489: Add NEWS and ACKS
https://hg.python.org/cpython/rev/8927417c5e88

New changeset 086c3e7a7955 by Zachary Ware in branch 'default':
Issue #19489: Merge with 3.5
https://hg.python.org/cpython/rev/086c3e7a7955
msg275420 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2016-09-09 20:16
Ammar, thank you very much for the patch!
History
Date User Action Args
2016-09-09 20:16:19zach.waresetmessages: + msg275420
2016-09-09 20:16:01python-devsetmessages: + msg275419
2016-09-09 20:12:09python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg275418

resolution: fixed
stage: patch review -> resolved
2016-09-09 20:05:20ammar2setfiles: + searchbar_in_header.diff4

messages: + msg275415
2016-09-03 16:14:34berker.peksagsetmessages: + msg274307
2016-09-03 12:33:47berker.peksagsetmessages: + msg274292
2016-09-03 04:38:19zach.waresetassignee: docs@python -> berker.peksag
messages: + msg274286
2016-08-24 15:55:30r.david.murraysetnosy: + r.david.murray

messages: + msg273574
stage: commit review -> patch review
2016-08-10 03:18:40ammar2setfiles: + searchbar_in_header.diff3

messages: + msg272296
2016-08-09 23:34:31berker.peksagsetmessages: + msg272279
2016-08-03 20:56:19zach.waresetversions: - Python 2.7
2016-08-03 20:51:37zach.waresetmessages: + msg271929
2016-08-03 20:39:10ammar2setfiles: + searchbar_in_header.diff2

messages: + msg271927
2016-08-03 20:19:09ammar2setfiles: + searchbar_in_header.diff

messages: + msg271925
2016-08-03 18:05:48zach.waresetmessages: + msg271918
stage: needs patch -> commit review
2016-07-24 01:28:17ammar2setfiles: + searchbar.diff2

messages: + msg271119
2016-07-23 19:52:31zach.waresetnosy: + zach.ware

messages: + msg271106
versions: + Python 2.7, Python 3.5, Python 3.6
2016-07-19 21:12:16ammar2setnosy: + eric.araujo
messages: + msg270843
2016-07-06 23:28:10ammar2setfiles: + searchbar.diff

nosy: + ammar2
messages: + msg269913

keywords: + patch
2016-06-05 19:02:43Jelle Zijlstrasetnosy: + Jelle Zijlstra
messages: + msg267460
2016-06-03 08:54:31BreamoreBoysetnosy: - BreamoreBoy
2016-06-03 03:53:38berker.peksagsetnosy: + berker.peksag
2014-10-02 23:17:04ezio.melottisetkeywords: + easy
nosy: + ezio.melotti

type: enhancement
stage: needs patch
2014-10-02 23:01:23BreamoreBoysetnosy: + BreamoreBoy
messages: + msg228278
2013-11-04 06:28:30georg.brandlcreate