v5.2.0: Migrate to DocSearch 3 #36176
Conversation
mdo
force-pushed
the
new-masthead
branch
3 times, most recently
from
4d069dd to
f09135b
Compare
| @@ -5,7 +5,7 @@ | |||
| {{- end }} | |||
|
|
|||
| {{ if eq .Page.Layout "docs" -}} | |||
| <script src="https://cdn.jsdelivr.net/npm/docsearch.js@2/dist/cdn/docsearch.min.js"></script> | |||
| <script src="https://cdn.jsdelivr.net/npm/@docsearch/js@3"></script> | |||
There was a problem hiding this comment.
We should serve this from npm at some point later along with all 3rd-party files.
XhmikosR
force-pushed
the
v520-docsearch3
branch
from
4ce3cbb to
6a9ee58
Compare
|
I added the query back because it's better; we only query it once, we guard the code with it and then we reuse it. |
| <form class="bd-search" data-shortcut="⌘K"> | ||
| <input type="search" class="form-control" id="search-input" placeholder="Search docs..." aria-label="Search docs for..." autocomplete="off" data-bd-docs-version="{{ .Site.Params.docs_version }}"> | ||
| </form> | ||
| <div class="bd-search" id="docsearch" data-bd-docs-version="{{ .Site.Params.docs_version }}"></div> |
There was a problem hiding this comment.
@patrickhlauke friendly ping to check if we are still OK with a11y :)
|
urgh, good lord, what have algolia done there... ok, it's not completely bad, but some right clangers. the dialog lacks a also, the oh-so-subtle text colour for the "No recent searches" and the keyboard shortcut/command hints is making my eyes go fuzzy...
is there an upstream repository where this can be fed back to them? had a quick look over the algolia site, but nothing jumped out at me directly... I'd say it's ok to ship it as is for now, but really hoping that we'll be able to punt them into an improved direction |
|
|
Pushed a fix for the color contrast as mentioned by @patrickhlauke, alignment issues noted by @julien-deramond, and added some custom styles to tweak the appearance. |
|
for what it's worth, filed this issue with algolia algolia/docsearch#1370 |
|
thanks @mdo the tweaks to colours/contrast look good |
|
going to mention this here (as not sure where else/if there's a more appropriate issue - if there's a more relevant one, feel free to copy/paste it over there): the dropdown control in the header to select different bootstrap versions should be a I'd tweak the styles so that focus outline is only suppressed when otherwise, navigating with keyboard, the focus indication currently is only a very faint change in colour, which is not sufficient. |
Co-authored-by: GeoSot <geo.sotis@gmail.com>
| @@ -62,4 +62,5 @@ <h1 class="bd-title mb-0" id="content">{{ .Title | markdownify }}</h1> | |||
| {{ range .Page.Params.extra_js -}} | |||
| <script{{ with .async }} async{{ end }} src="{{ .src }}"></script> | |||
| {{- end -}} | |||
| <div class="position-fixed"><input type="text"></div> | |||
There was a problem hiding this comment.
Sorry to disturb this merged PR, but this div/input looks useless to me. Is that a mistake or did I misunderstand something in here ?
There was a problem hiding this comment.
This is a hack to keep Safari from jumping to the bottom of the page. It's a docsearch bug that hasn't been fixed yet.
There was a problem hiding this comment.
Oh ok, thanks a lot for your answer !
This replaces our current DocSearch 2 implementation with DocSearch 3, including a new dialog that pops up that can save recent searches in local storage. I've lightly restyled things to better suit our docs, but could be down for more changes. Right now this is all done via CDN links, but we could also bring this in via npm or a manual copy-pasta if we prefer that.
Current known issue w/ Safari: algolia/docsearch#1260.
Base branch is #35736, but that'll change after we merge that.
Fixes #33338