Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Prevent duplicate HTML IDs on photo page #6356

Merged
merged 2 commits into from
Sep 22, 2023

Conversation

larseggert
Copy link
Collaborator

Fixes #6355

@codecov
Copy link

codecov bot commented Sep 22, 2023

Codecov Report

Merging #6356 (f38b2e2) into main (32bb4ef) will decrease coverage by 0.01%.
Report is 35 commits behind head on main.
The diff coverage is 87.83%.

@@            Coverage Diff             @@
##             main    #6356      +/-   ##
==========================================
- Coverage   88.69%   88.68%   -0.01%     
==========================================
  Files         290      290              
  Lines       40390    40422      +32     
==========================================
+ Hits        35823    35848      +25     
- Misses       4567     4574       +7     
Files Changed Coverage Δ
ietf/celeryapp.py 0.00% <0.00%> (ø)
ietf/help/views.py 63.63% <ø> (+8.46%) ⬆️
ietf/ipr/models.py 88.95% <ø> (ø)
ietf/urls.py 80.00% <ø> (-0.56%) ⬇️
ietf/ipr/views.py 87.96% <95.45%> (+0.24%) ⬆️
ietf/doc/models.py 88.98% <100.00%> (ø)
ietf/doc/utils.py 87.38% <100.00%> (+0.23%) ⬆️
ietf/doc/views_review.py 95.16% <100.00%> (+0.20%) ⬆️
ietf/doc/views_search.py 89.09% <100.00%> (-0.21%) ⬇️
ietf/ipr/feeds.py 93.33% <100.00%> (+0.22%) ⬆️
... and 3 more

... and 2 files with indirect coverage changes

@rjsparks
Copy link
Member

This will break anyone that is used to typing http://localhost:8000/wg/photos/#h to get to the h's.
http://localhost:8000/wg/photos/#h8 is just not something they're going to guess.

It also just pushes the problem down the road (this problem exists on the current page) on doing the right thing with real names. Someone with a non [a-zA-Z] first character will end up with a section that is labelled with an [a-zA-Z] heading that is sorted in nothing matching any standardized collation. Are ids restricted to [a-zA-Z]? Could we just use the first unicode character instead and make sure the results are sorted with the system's collation setting? This template is only used for working group chairs, so we're not likely to have random garbage thrown at production.

@larseggert
Copy link
Collaborator Author

larseggert commented Sep 22, 2023

We could. Let me see if I can rework this.

@larseggert larseggert closed this Sep 22, 2023
@larseggert larseggert reopened this Sep 22, 2023
@larseggert larseggert marked this pull request as draft September 22, 2023 14:06
@larseggert larseggert marked this pull request as ready for review September 22, 2023 14:31
@larseggert
Copy link
Collaborator Author

Turns out HTML5 IDs can be Unicode :-)

@rjsparks rjsparks merged commit 910a266 into ietf-tools:main Sep 22, 2023
9 checks passed
@larseggert larseggert deleted the fix-6355 branch September 23, 2023 06:01
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate HTML ID intemplates/group/all_photos.html
2 participants