Skip to content

Commit 924aeb1

Browse files
roed-mathroed-math
committed
Fix PR 7009 review findings
1 parent 0af55ac commit 924aeb1

10 files changed

Lines changed: 138 additions & 30 deletions

File tree

‎lmfdb/ecnf/templates/ecnf-index.html‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22

33
{% block content %}
44

5+
{{ tabs(
6+
('Elliptic curves over $\\Q$', 'ec.rational_elliptic_curves', false),
7+
('Elliptic curves over number fields', 'ecnf.index', true)
8+
) }}
9+
510
<div>
611
{{ info.stats.short_summary | safe}}
712
</div>

‎lmfdb/elliptic_curves/templates/ec-index.html‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22

33
{% block content %}
44

5+
{{ tabs(
6+
('Elliptic curves over $\\Q$', 'ec.rational_elliptic_curves', true),
7+
('Elliptic curves over number fields', 'ecnf.index', false)
8+
) }}
9+
510
<div>
611
{{info.stats.short_summary| safe}}
712
</div>

‎lmfdb/elliptic_curves/test_browse_page.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,18 @@ def test_page(self):
1212
homepage = self.tc.get("/EllipticCurve/Q/").get_data(as_text=True)
1313
assert 'Label or coefficients' in homepage
1414

15+
def test_tabs(self):
16+
r"""Check that both elliptic-curve landing pages render reciprocal tabs."""
17+
q_page = self.tc.get("/EllipticCurve/Q/").get_data(as_text=True)
18+
assert 'class="tab-container"' in q_page
19+
assert r'<span class="tab-active">Elliptic curves over $\Q$</span>' in q_page
20+
assert '<a href="/EllipticCurve/">Elliptic curves over number fields</a>' in q_page
21+
22+
nf_page = self.tc.get("/EllipticCurve/").get_data(as_text=True)
23+
assert 'class="tab-container"' in nf_page
24+
assert '<span class="tab-active">Elliptic curves over number fields</span>' in nf_page
25+
assert r'<a href="/EllipticCurve/Q/">Elliptic curves over $\Q$</a>' in nf_page
26+
1527
#
1628
# Link to stats page
1729
def test_stats(self):

‎lmfdb/groups/abstract/main.py‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -781,9 +781,25 @@ def index():
781781
if request.args:
782782
search_types = request.args.getlist("search_type")
783783
info["search_type"] = search_type = search_types[-1] if search_types else info.get("hst", "")
784-
# If only search_type is given (no actual search params), show the landing page
785784
if search_type in ["List", "", "Random", "Diagram"]:
786785
return group_search(info)
786+
# Preserve old abstract-group search URLs while directing users to the
787+
# new, object-specific landing pages. Keep Random* as a search type so
788+
# that SearchWrapper still performs a random lookup on the new route.
789+
legacy_searches = {
790+
"Subgroups": (".sub_index", None),
791+
"RandomSubgroup": (".sub_index", "RandomSubgroup"),
792+
"ComplexCharacters": (".char_index", None),
793+
"RandomComplexCharacter": (".char_index", "RandomComplexCharacter"),
794+
"ConjugacyClasses": (".conjugacy_class_index", None),
795+
}
796+
if search_type in legacy_searches:
797+
endpoint, new_search_type = legacy_searches[search_type]
798+
args = request.args.to_dict(flat=False)
799+
args.pop("search_type", None)
800+
if new_search_type is not None:
801+
args["search_type"] = [new_search_type]
802+
return redirect(url_for(endpoint, **args), 307)
787803
info["stats"] = GroupStats()
788804
info["count"] = 50
789805
info["order_list"] = ["1-64", "65-127", "128", "129-255", "256", "257-383", "384", "385-511", "513-1000", "1001-1500", "1501-2000", "2001-"]
@@ -831,7 +847,7 @@ def sub_index():
831847
("central=yes", "central"),
832848
("perfect=yes", "perfect"),
833849
("characteristic=yes", "characteristic"),
834-
]
850+
]
835851
info["stats"] = GroupStats()
836852
info["search_array"] = SubgroupSearchArray()
837853
info["count"] = 50

‎lmfdb/groups/abstract/stats.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,4 +232,3 @@ def char_summary(self):
232232
@lazy_attribute
233233
def cc_summary(self):
234234
return fr'The database currently contains {comma(db.gps_conj_classes.count())} {display_knowl("group.conjugacy_class", "conjugacy classes")} from among {comma(db.gps_groups.count({"conjugacy_classes_known":True}))} different {display_knowl("group", "groups")}. You can <a href="{url_for(".statistics")}">browse further statistics</a>.'
235-

‎lmfdb/groups/abstract/test_browse_page.py‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from urllib.parse import parse_qs, urlsplit
2+
13
from lmfdb.tests import LmfdbTest
24

35
## TODO
@@ -14,6 +16,32 @@ def test_index_page(self):
1416
homepage = self.tc.get("/Groups/Abstract/").get_data(as_text=True)
1517
assert "database currently contains" in homepage
1618

19+
def test_legacy_search_urls(self):
20+
r"""
21+
Check that old search URLs redirect to the new landing pages without
22+
dropping their filters.
23+
"""
24+
cases = [
25+
("/Groups/Abstract/?search_type=Subgroups&ambient=128.207",
26+
"/Groups/Abstract/Subgroups", {"ambient": ["128.207"]}),
27+
("/Groups/Abstract/?search_type=RandomSubgroup&ambient=128.207",
28+
"/Groups/Abstract/Subgroups",
29+
{"ambient": ["128.207"], "search_type": ["RandomSubgroup"]}),
30+
("/Groups/Abstract/?search_type=ComplexCharacters&dim=3",
31+
"/Groups/Abstract/ComplexCharacters", {"dim": ["3"]}),
32+
("/Groups/Abstract/?search_type=RandomComplexCharacter&dim=3",
33+
"/Groups/Abstract/ComplexCharacters",
34+
{"dim": ["3"], "search_type": ["RandomComplexCharacter"]}),
35+
("/Groups/Abstract/?search_type=ConjugacyClasses&group=12.4",
36+
"/Groups/Abstract/ConjugacyClasses", {"group": ["12.4"]}),
37+
]
38+
for source, expected_path, expected_query in cases:
39+
response = self.tc.get(source)
40+
target = urlsplit(response.location)
41+
assert response.status_code == 307
42+
assert target.path == expected_path
43+
assert parse_qs(target.query) == expected_query
44+
1745
# TODO test stats once we have them
1846
# def test_stats_page(self):
1947
# self.check_args("/Groups/Abstract/stats","Abstract groups: Statistics")

‎lmfdb/groups/abstract/web_groups.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1108,7 +1108,7 @@ def subgp_paragraph(self):
11081108
if self.number_subgroups is None:
11091109
if self.number_normal_subgroups is None:
11101110
return " "
1111-
elif self.number_characteristic_subgroups is None:
1111+
elif self.number_characteristic_subgroups is None:
11121112
return """There are <a href=" """ + str(url_for('.sub_index', ambient=self.label, normal='yes')) + """ "> """ + str(self.number_normal_subgroups) + " normal</a> subgroups. <p>"+normalcolor
11131113
else:
11141114
ret_str = """ There are <a href=" """ + str(url_for('.sub_index', ambient=self.label, normal='yes')) + """ "> """ + str(self.number_normal_subgroups) + """ normal subgroups</a>"""

‎lmfdb/static/lmfdb.js‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,25 +36,39 @@ $.fn.round_bl = function(val) {
3636
$(function() {
3737
/* Add a chevron to each properties-header that has a following properties-body,
3838
wired to collapse/expand only that section. */
39-
$('.properties-header').each(function() {
39+
$('.properties-header').each(function(index) {
4040
var $header = $(this);
4141
var $body = $header.next('.properties-body');
4242
if ($body.length === 0) return;
4343

44-
var $chev = $('<span class="prop-chevron open"></span>');
44+
var title = $.trim($header.text());
45+
var bodyId = $body.attr('id') || 'lmfdb-properties-body-' + index;
46+
$body.attr('id', bodyId);
47+
48+
var $chev = $('<button type="button" class="prop-chevron open"></button>');
49+
$chev.attr('aria-controls', bodyId);
4550
$header.append($chev);
4651

47-
var key = 'lmfdb_prop_' + $.trim($header.text()).replace(/\s+/g, '_').substring(0, 30);
52+
function setExpanded(expanded) {
53+
$chev.toggleClass('open', expanded).toggleClass('closed', !expanded);
54+
$chev.attr('aria-expanded', expanded ? 'true' : 'false');
55+
$chev.attr('aria-label', (expanded ? 'Collapse ' : 'Expand ') + title);
56+
}
57+
58+
var key = 'lmfdb_prop_' + title.replace(/\s+/g, '_').substring(0, 30);
4859
if (localStorage.getItem(key) === '0') {
4960
$body.hide();
50-
$chev.removeClass('open').addClass('closed');
61+
setExpanded(false);
62+
} else {
63+
setExpanded(true);
5164
}
5265

5366
$chev.on('click', function(e) {
5467
e.stopPropagation();
5568
e.preventDefault();
56-
$chev.toggleClass('open closed');
57-
localStorage.setItem(key, $chev.hasClass('open') ? '1' : '0');
69+
var expanded = $chev.attr('aria-expanded') !== 'true';
70+
setExpanded(expanded);
71+
localStorage.setItem(key, expanded ? '1' : '0');
5872
$body.slideToggle(150);
5973
});
6074
});

‎lmfdb/templates/sidebar.html‎

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -85,22 +85,33 @@
8585
<script>
8686
$(document).ready(function() {
8787

88+
function setToggleState($toggle, expanded, label) {
89+
$toggle.toggleClass('open', expanded);
90+
$toggle.attr('aria-expanded', expanded ? 'true' : 'false');
91+
$toggle.attr('aria-label', (expanded ? 'Collapse ' : 'Expand ') + label);
92+
}
93+
8894
// ── Collapsible sub-menu toggles (Elliptic curves, Abstract groups, …) ──
89-
$('.collapsible-container').each(function() {
95+
$('.collapsible-container').each(function(index) {
9096
var $container = $(this);
9197
var $content = $container.find('.collapsible-content');
92-
var $chev = $('<span class="collapsible-chevron open"><svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"><path d="M9 6L15 12L9 18" stroke="currentColor" stroke-width="3"/></svg></span>');
93-
$content.before($chev);
94-
9598
// Key from the container's OWN text only, excluding child elements
96-
var ownText = $container.clone().children().remove().end().text();
99+
var ownText = $.trim($container.clone().children().remove().end().text());
97100
var key = 'lmfdb_sidebar_entry_' +
98-
$.trim(ownText).replace(/\s+/g, '_').substring(0, 30);
101+
ownText.replace(/\s+/g, '_').substring(0, 30);
102+
var contentId = 'lmfdb-sidebar-entry-' + index;
103+
$content.attr('id', contentId);
104+
105+
var $chev = $('<button type="button" class="collapsible-chevron open"><svg aria-hidden="true" focusable="false" width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"><path d="M9 6L15 12L9 18" stroke="currentColor" stroke-width="3"/></svg></button>');
106+
$chev.attr('aria-controls', contentId);
107+
$content.before($chev);
99108

100109
// Open by default; collapse only if explicitly stored closed.
101110
if (localStorage.getItem(key) === '0') {
102111
$content.hide();
103-
$chev.removeClass('open');
112+
setToggleState($chev, false, ownText);
113+
} else {
114+
setToggleState($chev, true, ownText);
104115
}
105116

106117
$container.on('click', function(e) {
@@ -109,43 +120,52 @@
109120

110121
e.stopPropagation();
111122
e.preventDefault();
112-
$chev.toggleClass('open');
123+
var expanded = $chev.attr('aria-expanded') !== 'true';
124+
setToggleState($chev, expanded, ownText);
113125
$content.slideToggle(150);
114-
localStorage.setItem(key, $chev.hasClass('open') ? '1' : '0');
126+
localStorage.setItem(key, expanded ? '1' : '0');
115127
});
116128
});
117129

118130
// ── Section (h2) collapse/expand ─────────────────────────────────────────
119-
$('#sidebar h2').each(function() {
131+
$('#sidebar h2').each(function(index) {
120132
var $h2 = $(this);
121133
var $content = $h2.nextUntil('h2').not('script');
122134
if ($content.length === 0) return; // 'single' type – nothing to collapse
135+
var headingText = $.trim($h2.text());
123136

124137
// Wrap all content between this h2 and the next into one div
125138
$content.wrapAll('<div class="section-content"></div>');
126139
var $section = $h2.next('.section-content');
140+
var sectionId = 'lmfdb-sidebar-section-' + index;
141+
$section.attr('id', sectionId);
127142

128143
// Append the chevron into the h2
129-
var $chev = $('<span class="collapsible-chevron open"><svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"><path d="M9 6L15 12L9 18" stroke="currentColor" stroke-width="3"/></svg></span>');
144+
var $chev = $('<button type="button" class="collapsible-chevron open"><svg aria-hidden="true" focusable="false" width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"><path d="M9 6L15 12L9 18" stroke="currentColor" stroke-width="3"/></svg></button>');
145+
$chev.attr('aria-controls', sectionId);
130146
$h2.append($chev);
131147
$h2.css('cursor', 'pointer');
132148

133149
// Restore persisted state (default: open)
134-
var key = 'lmfdb_sidebar_sec_' + $.trim($h2.text()).replace(/\s+/g, '_').substring(0, 30);
150+
var key = 'lmfdb_sidebar_sec_' + headingText.replace(/\s+/g, '_').substring(0, 30);
135151
if (localStorage.getItem(key) === '0') {
136152
$section.hide();
137-
$chev.removeClass('open');
153+
setToggleState($chev, false, headingText);
154+
} else {
155+
setToggleState($chev, true, headingText);
138156
}
139157

140158
$h2.on('click', function(e) {
159+
// Preserve any heading link that may be added in the future.
160+
if ($(e.target).closest('a').length) return;
141161
e.stopPropagation();
142162
e.preventDefault();
143-
$chev.toggleClass('open');
163+
var expanded = $chev.attr('aria-expanded') !== 'true';
164+
setToggleState($chev, expanded, headingText);
144165
$section.slideToggle(150);
145-
localStorage.setItem(key, $chev.hasClass('open') ? '1' : '0');
166+
localStorage.setItem(key, expanded ? '1' : '0');
146167
});
147168
});
148169

149170
});
150171
</script>
151-

‎lmfdb/templates/style.css‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,11 @@ body > .debug {
395395
flex-shrink: 0;
396396
cursor: pointer;
397397
color: {{ color.properties_header_text }};
398-
padding-left: 10px;
398+
background: transparent;
399+
border: 0;
400+
border-radius: 0;
401+
min-width: 0;
402+
padding: 4px 2px 4px 10px;
399403
line-height: 1;
400404
}
401405
.prop-chevron::before {
@@ -1618,9 +1622,15 @@ div.maassformplot img {
16181622
flex-shrink: 0;
16191623
cursor: pointer;
16201624
color: {{color.col_sidebar_header_links}};
1625+
background: transparent;
1626+
border: 0;
1627+
border-radius: 0;
1628+
min-width: 0;
1629+
padding: 0;
16211630
margin-right: 4px;
16221631
margin-left: auto;
1623-
align-items: right;
1632+
display: flex;
1633+
align-items: center;
16241634
}
16251635

16261636

@@ -1630,7 +1640,6 @@ div.maassformplot img {
16301640
object-fit: contain;
16311641
transform: rotate(0deg);
16321642
transition: transform 0.15s ease;
1633-
vertical-align: center;
16341643
}
16351644

16361645
#sidebar .collapsible-chevron.open > svg {
@@ -1660,7 +1669,7 @@ div.maassformplot img {
16601669
/* No right margin so the chevrons are aligned */
16611670
padding: 4px 0px 4px 4px;
16621671
/* padding-left: 5px; */
1663-
1672+
16641673
}
16651674
#sidebar .future {
16661675
color: {{color.sidebar_text_future}};

0 commit comments

Comments
 (0)