Skip to content

Commit c59d0cd

Browse files
committed
Restored search hotkey for desktop.
1 parent b1a45b7 commit c59d0cd

File tree

13 files changed

+158
-34
lines changed

13 files changed

+158
-34
lines changed

.github/workflows/tests.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ jobs:
3939
run: |
4040
python -m pip install --upgrade pip setuptools
4141
python -m pip install -r requirements/tests.txt
42+
- name: Install playwright browsers
43+
run: |
44+
python -m playwright install --with-deps
4245
- name: Set up databases
4346
run: |
4447
PGPASSWORD="postgres" createuser -U postgres -d djangoproject --superuser -h localhost

Dockerfile

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ COPY ./requirements ./requirements
3131
RUN apt-get update \
3232
&& apt-get install --assume-yes --no-install-recommends ${BUILD_DEPENDENCIES} \
3333
&& python3 -m pip install --no-cache-dir -r ${REQ_FILE} \
34+
&& if [ "${REQ_FILE}" = "requirements/tests.txt" ]; then \
35+
echo "Installing Playwright browsers..."; \
36+
playwright install --with-deps; \
37+
fi \
3438
&& apt-get purge --assume-yes --auto-remove ${BUILD_DEPENDENCIES} \
3539
&& apt-get distclean
3640

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ APP_LIST ?= accounts aggregator blog contact dashboard djangoproject docs founda
44
SCSS = djangoproject/scss
55
STATIC = djangoproject/static
66

7-
ci: compilemessages test
7+
ci: compilemessages collectstatics test
88
@python -m coverage report
99

1010
compilemessages:

djangoproject/static/js/djangoproject.js

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,16 +67,18 @@ document.querySelector('.menu-button').addEventListener('click', function () {
6767

6868
// Update search input placeholder text based on the user's operating system
6969
(function () {
70-
const el = document.getElementById('id_q');
70+
const inputs = [
71+
document.getElementById('id_desktop-q'),
72+
document.getElementById('id_mobile-q'),
73+
];
7174

72-
if (!el) {
73-
return;
74-
}
75+
const el = inputs.find((el) => el.checkVisibility());
76+
if (!el) return;
7577

76-
const original_placeholder = el.getAttribute('placeholder');
7778
const is_mac = navigator.userAgent.indexOf('Mac') !== -1;
78-
const new_value = `${original_placeholder} (${is_mac ? '⌘\u200aK' : 'Ctrl+K'})`;
7979

80+
const original_placeholder = el.getAttribute('placeholder');
81+
const new_value = `${original_placeholder} (${is_mac ? '⌘\u200aK' : 'Ctrl+K'})`;
8082
el.setAttribute('placeholder', new_value);
8183
})();
8284

@@ -94,7 +96,13 @@ window.addEventListener('keydown', function (e) {
9496

9597
e.preventDefault();
9698

97-
const el = document.querySelector('#id_q');
99+
const inputs = [
100+
document.getElementById('id_desktop-q'),
101+
document.getElementById('id_mobile-q'),
102+
];
103+
104+
const el = inputs.find((el) => el.checkVisibility());
105+
if (!el) return;
98106

99107
el.select();
100108
el.focus();

djangoproject/templates/includes/header.html

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
{% load docs %}
2+
{% search_context as cached_search_context %}
23
{% if 'preview.djangoproject.com' in request.get_host %}
34
<div class="copy-banner" style="background: #fff78e; padding: 10px;"></div>
45
{% endif %}
@@ -7,7 +8,7 @@
78
<a class="logo" href="{% url 'homepage' %}">Django</a>
89
<p class="meta">The web framework for perfectionists with deadlines.</p>
910
<div class="header-mobile-only">
10-
{% search_form %}
11+
{% search_form prefix="mobile" cached_search_context=cached_search_context %}
1112
<div class="light-dark-mode-toggle">
1213
{% include "includes/toggle_theme.html" %}
1314
</div>
@@ -47,7 +48,7 @@
4748
<a href="{% url 'fundraising:index' %}">&#9829; Donate</a>
4849
</li>
4950
<li>
50-
{% search_form %}
51+
{% search_form prefix="desktop" cached_search_context=cached_search_context %}
5152
</li>
5253
<li>
5354
{% include "includes/toggle_theme.html" %}

djangoproject/templates/search_form.html

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1-
{% load i18n %}
1+
{% load i18n docs %}
22
<search class="search form-input" aria-labelledby="docs-search-label">
3-
<form action="{% url 'document-search' version=version lang=lang host 'docs' %}">
3+
<form action="{% url 'document-search' version=cached_search_context.version lang=cached_search_context.lang host 'docs' %}">
4+
{% build_search_form cached_search_context.release prefix as form %}
45
<label id="docs-search-label" class="visuallyhidden" for="{{ form.q.id_for_label }}">{{ form.q.field.widget.attrs.placeholder }}</label>
56
{{ form.q }}
6-
<input type="hidden" name="category" value="{{ active_category }}">
7+
<input type="hidden" name="category" value="{{ cached_search_context.active_category }}">
78

89
<button type="submit">
910
<i class="icon icon-search" aria-hidden="true"></i>

djangoproject/tests.py

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
1+
import os
12
from http import HTTPStatus
23
from io import StringIO
34

45
from django.conf import settings
6+
from django.contrib.staticfiles.testing import StaticLiveServerTestCase
57
from django.core.management import call_command
68
from django.test import TestCase
79
from django.urls import NoReverseMatch, get_resolver
810
from django.utils.translation import activate, gettext as _
911
from django_hosts.resolvers import reverse
12+
from playwright.sync_api import expect, sync_playwright
1013

1114
from docs.models import DocumentRelease, Release
1215

@@ -211,3 +214,70 @@ def test_single_h1_per_page(self):
211214
response = self.client.get(url)
212215
self.assertEqual(response.status_code, 200)
213216
self.assertContains(response, "<h1", count=1)
217+
218+
219+
class EndToEndTests(ReleaseMixin, StaticLiveServerTestCase):
220+
@classmethod
221+
def setUpClass(cls):
222+
os.environ["DJANGO_ALLOW_ASYNC_UNSAFE"] = "true"
223+
super().setUpClass()
224+
cls.playwright = sync_playwright().start()
225+
cls.browser = cls.playwright.chromium.launch()
226+
227+
@classmethod
228+
def tearDownClass(cls):
229+
super().tearDownClass()
230+
cls.browser.close()
231+
cls.playwright.stop()
232+
233+
def setUp(self):
234+
self.setUpTestData()
235+
self.mac_user_agent = "Mozilla/5.0 (Macintosh) AppleWebKit"
236+
self.windows_user_agent = "Mozilla/5.0 (Windows NT 10.0)"
237+
self.mobile_linux_user_agent = "Mozilla/5.0 (Linux; Android 10; Mobile)"
238+
239+
def test_search_ctrl_k_hotkey_desktop(self):
240+
page = self.browser.new_page(user_agent=self.windows_user_agent)
241+
page.goto(self.live_server_url)
242+
243+
mobile_search_bar = page.locator("#id_mobile-q")
244+
desktop_search_bar = page.locator("#id_desktop-q")
245+
self.assertFalse(mobile_search_bar.is_visible())
246+
self.assertTrue(desktop_search_bar.is_visible())
247+
expect(desktop_search_bar).to_have_attribute("placeholder", "Search (Ctrl+K)")
248+
is_focused = page.evaluate("document.activeElement.id === 'id_desktop-q'")
249+
self.assertFalse(is_focused)
250+
251+
page.keyboard.press("Control+KeyK")
252+
is_focused = page.evaluate("document.activeElement.id === 'id_desktop-q'")
253+
self.assertTrue(is_focused)
254+
page.close()
255+
256+
def test_search_ctrl_k_hotkey_mobile(self):
257+
page = self.browser.new_page(
258+
user_agent=self.mobile_linux_user_agent,
259+
viewport={"width": 375, "height": 812},
260+
)
261+
page.goto(self.live_server_url)
262+
263+
mobile_search_bar = page.locator("#id_mobile-q")
264+
desktop_search_bar = page.locator("#id_desktop-q")
265+
self.assertTrue(mobile_search_bar.is_visible())
266+
expect(mobile_search_bar).to_have_attribute("placeholder", "Search (Ctrl+K)")
267+
self.assertFalse(desktop_search_bar.is_visible())
268+
is_focused = page.evaluate("document.activeElement.id === 'id_mobile-q'")
269+
self.assertFalse(is_focused)
270+
271+
page.keyboard.press("Control+KeyK")
272+
is_focused = page.evaluate("document.activeElement.id === 'id_mobile-q'")
273+
self.assertTrue(is_focused)
274+
page.close()
275+
276+
def test_search_placeholder_mac_mode(self):
277+
page = self.browser.new_page(user_agent=self.mac_user_agent)
278+
page.goto(self.live_server_url)
279+
280+
desktop_search_bar = page.locator("#id_desktop-q")
281+
expect(desktop_search_bar).to_have_attribute("placeholder", "Search (⌘\u200aK)")
282+
283+
page.close()

djangoproject/utils.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,24 @@
22

33

44
class CachedLibrary(template.Library):
5-
def cached_context_inclusion_tag(self, template_name, *, name=None):
5+
def cached_context_simple_tag(self, name=None):
66
"""
7-
Wraps @register.inclusion_tag(template_name, takes_context=True) to
8-
automatically cache the returned context dictionary inside the
9-
template's render_context for the duration of a single render pass.
7+
Wraps @register.simple_tag(takes_context=True) to cache the returned
8+
value inside the template's render_context during a single template
9+
render pass.
1010
1111
This is useful when a tag may be rendered multiple times within the
12-
same template and computing its context is expensive (e.g. due to
13-
database queries).
12+
same template and with an expensive computation (e.g. due to database
13+
queries).
1414
"""
1515

1616
def decorator(func):
1717
tag_name = name or func.__name__
1818

19-
@self.inclusion_tag(template_name, takes_context=True, name=tag_name)
19+
@self.simple_tag(takes_context=True, name=tag_name)
2020
def wrapper(context, *args, **kwargs):
2121
render_context = getattr(context, "render_context", None)
22-
cache_key = f"{tag_name}_cached_context"
22+
cache_key = f"{tag_name}_cached_value"
2323

2424
if render_context is not None and cache_key in render_context:
2525
return render_context[cache_key]

docs/forms.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,8 @@ def __init__(self, data=None, **kwargs):
1616
"placeholder": search_label_placeholder,
1717
}
1818
)
19+
q_with_prefix = super().add_prefix("q")
20+
self.fields["q"].widget.attrs["id"] = f"id_{q_with_prefix}"
21+
22+
def add_prefix(self, field_name):
23+
return field_name

docs/templatetags/docs.py

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,36 @@
2020
register = CachedLibrary()
2121

2222

23-
@register.cached_context_inclusion_tag("search_form.html")
24-
def search_form(context):
25-
if "request" not in context:
23+
@register.inclusion_tag("search_form.html", takes_context=True)
24+
def search_form(context, *, prefix, cached_search_context=None):
25+
return {
26+
"prefix": prefix,
27+
"request": context.get("request"),
28+
"cached_search_context": cached_search_context,
29+
}
30+
31+
32+
@register.simple_tag(takes_context=True)
33+
def build_search_form(context, release, prefix=None):
34+
request = context.get("request")
35+
return DocSearchForm(
36+
request.GET if request else None, release=release, prefix=prefix
37+
)
38+
39+
40+
@register.cached_context_simple_tag()
41+
def search_context(context):
42+
if context.get("request") is None:
2643
# Django's built-in error views (like django.views.defaults.server_error)
2744
# render templates without attaching a RequestContext — meaning the 'request'
2845
# variable is not present in the template context.
2946
return {
30-
"form": DocSearchForm(release=None),
3147
"version": "dev",
3248
"lang": settings.DEFAULT_LANGUAGE_CODE,
49+
"release": None,
50+
"active_category": "",
3351
}
3452

35-
request = context["request"]
3653
lang = context.get("lang", settings.DEFAULT_LANGUAGE_CODE)
3754
active_category = context.get("active_category", "")
3855

@@ -44,10 +61,10 @@ def search_form(context):
4461
release = DocumentRelease.objects.select_related("release").current()
4562

4663
return {
47-
"form": DocSearchForm(request.GET, release=release),
4864
"version": release.version,
4965
"lang": lang,
5066
"active_category": active_category,
67+
"release": release,
5168
}
5269

5370

0 commit comments

Comments
 (0)