Search Page Implementation & Review Fixes by kurapativyshnavi · Pull Request #205 · wpaccessibility/wp-a11y-docs

kurapativyshnavi · 2025-10-18T08:02:50Z

I've implemented the dedicated search results page and resolved all code issues from the review.

What was changed:

search.html: Added this new file for the search results page layout.
_includes/components/search_header.html: Added the new search input and the search-submit button markup.
_sass/components/_search.scss: Updated the styles. I used Flexbox to line up the search input and button, and added CSS to make the submit button visible with an accessible :focus state.
assets/js/wp-a11y-docs.js: Changed the document ready function to jtd.onReady().

Added logic to re-populate the search box with the URL query when the page loads.

Implemented code to wait for the search index to finish loading before running the search, resolving the timing issue.

Thanks for the review!

…iew notes.

rianrietveld · 2025-10-20T05:55:14Z

@kurapativyshnavi thanks!
@joedolson can you review this when you have time?
I know you are busy the next few days, when you have time :-) .

This is a copy of the file from PR 90.

joedolson

There are a number of changes required to make this functional.

github-actions · 2025-10-23T18:15:41Z

PR Preview
Preview removed because the pull request was closed.
2025-11-13 06:30 UTC

…f search results.

… into pr/205

This doesn't update after the initial load, and shouldn't be live.

This isn't readily internationalizable, but none of the theme text is, so that's probably not a thing to worry about.

Makes this reusable.

- Close search results if focus leaves container - Let arrow keys actually move focus and execute arrow key support within the results instead of only from the search field

obsolete

rianrietveld

Yes, it works, thank you!

A few things:

The Skip link brings you to the first link in the content, skipping the search.
2 ways to solve this:

add a skip to search or
let the skip link move to the search input field.

This is personal, but I really hate the  in your face 1999 styling.
Please remove that yellow and only leave bold in the CSS for mark.

There is an empty  and .

<div class="search-result-section">
<mark></mark>
<mark class="search-result-highlight">WCAG Succes Criteria</mark>
<span></span>
</div>

With the mobile search, I find it unexpected that the focus scrolls up to the top of the window. We can wait and see what the auditor says about that.

Suggestion:
We can add a description to the pages and use that instead of the uri below the link.

Question:
DId you add pagination?

And note to self:
The headings must be more descriptive, search for WCAG gives multiple times the same vage results. I will fix that.

rianrietveld · 2025-11-10T09:15:16Z

      searchResults.appendChild(resultsList);
-      screenReaderFeedback.innerText = results.length + ' results found, tab to read them';
+      if ( results.length === 1 ) {
+        screenReaderFeedback.innerText = results.length + ' result found, tab to read it';


'tab to read it' is in fact 'arrow down to read it', a tab brings you to the search button.

Close the list of search results when the focus moves to the search button.

Currently, you can navigate to the search results either using tab or using arrows. Before this PR, it was only by tab. Do we want to only support arrows? In that case, I'd want to also add some visual affordance so that's more obvious.

No, it's about the tab order and the instruction text. You go from input to search buttons to search results. And then the text "tab to read it" doesn't make sense.

But if you change the tab order, one is forced to tab through all the results to get to the submit button. and that's no use either.

In fact, how it works now is great, but if you cannot see, the focus on the submit button instead of the results could be confusing. Am I overthinking this?

So, I was responding to your second comment, not the first. The text change makes sense; but if we close the list of search results when focus moves to the search button, then a user can't get to the search results using the tab key.

rianrietveld · 2025-11-10T10:16:35Z

Additional: If you press enter and not the search button to submit the search, the {{site.baseurl}} is not attached in the form submit and it gives a 404

Appears to be a bug in just the docs.

I'm assuming that some configs of the theme do partial word highlighting? Not sure.

joedolson · 2025-11-13T00:02:19Z

I've dealt with most things here, and the ones I haven't addressed are things I think should be handled in a follow up, if needed.

Did not change the Skip link behavior. First, because I think it's unrelated to the search changes and should be done separately if at all, and second because I'm not convinced it's a good change. The skip link bypassing the search makes sense to me; it's not part of the content. But the language of the skip link isn't correct, as it's not skipping the menu; it's skipping to the content. I agree that a change needs to be made.
Did not remove the mark styling. I think all style characteristics should be handled as part of the redesign effort.
Did not add description to the search index. That's just because that's really complicated, and I think we should get this finished as it is and then iterate to add those indices.
Did not add pagination. Same reason: pretty complicated, and I think we should get this iteration done first.
Did not change so that the list of search results closes when focus moves to the search button. I think that would make this less predictable, and it's better that users can reach the results either by arrows or by tabbing.

rianrietveld

@joedolson
I agree with your choices, let’s merge this now.
Thanks so much for your work!

Implement full search results page. Includes fixes for CSS and JS rev…

46cdaa4

…iew notes.

rianrietveld requested a review from joedolson October 20, 2025 05:53

Add missing search.html template

0f40b87

This is a copy of the file from PR 90.

joedolson previously requested changes Oct 21, 2025

View reviewed changes

Comment thread _sass/components/_search.scss Outdated

Comment thread _sass/components/_search.scss Outdated

Comment thread _sass/components/_search.scss Outdated

Comment thread _sass/components/_search.scss Outdated

Comment thread assets/js/wp-a11y-docs.js Outdated

joedolson mentioned this pull request Oct 21, 2025

Improve accessibility of the search option by adding results page #90

Closed

joedolson added 3 commits October 23, 2025 13:05

Add query for search results page.

b9507f0

Remove undefined mixin and border radius

931129b

Merge branch 'main' into pr/205

0b71fa6

joedolson added 2 commits October 23, 2025 13:19

Re-add z-index so label stacks above input

8d5bc9b

Make button visibility global, integrate styles, improve visibility o…

bbead6a

…f search results.

rianrietveld linked an issue Oct 25, 2025 that may be closed by this pull request

Improve the accessibility of the search option #2

Closed

joedolson added 16 commits October 25, 2025 11:27

Merge branch 'main' into main

ba4a070

Merge branch 'main' into pr/205

f80c276

Merge branch 'main' of https://github.com/kurapativyshnavi/wp-a11y-docs…

454b792

… into pr/205

Add site.baseurl to form action; remove unnecessary aria-label

8d676d4

Remove relative URL from autocomplete.

d93e6d2

Add section to search page results

07a82b2

Exclude from nav; remove aria-live.

06d85c3

This doesn't update after the initial load, and shouldn't be live.

change no of results to heading, change list to ordered.

3ba0fe5

Add padding around search results

1d5e6c7

doc and section were reversed.

efe4f30

Fix autocomplete sizing across device sizes

8bf129f

Make search results response use correct pluralization

1bb7269

This isn't readily internationalizable, but none of the theme text is, so that's probably not a thing to worry about.

HTML tweak; clearly label page vs. section results; stripe results.

117c64b

Move keyboard handling into function

e2ff39b

Makes this reusable.

Improvements to search complete nav

c06eadc

- Close search results if focus leaves container - Let arrow keys actually move focus and execute arrow key support within the results instead of only from the search field

Update wp-a11y-docs.js

920abe1

joedolson requested a review from rianrietveld November 5, 2025 18:11

rianrietveld requested changes Nov 10, 2025

View reviewed changes

joedolson added 7 commits November 12, 2025 17:19

Merge branch 'main' into pr/205

9eb40ed

Remove duplicate mark

433e22b

Appears to be a bug in just the docs.

Remove empty span

28c7b7e

I'm assuming that some configs of the theme do partial word highlighting? Not sure.

Change instructional text.

45c1944

Update search.html

fe2a916

Manipulate page title and h1 to modify results based on search.

871d3c2

Remove unnecessary key handler for submitting search.

ff288ae

rianrietveld self-requested a review November 13, 2025 06:29

rianrietveld approved these changes Nov 13, 2025

View reviewed changes

rianrietveld merged commit 51357dd into wpaccessibility:main Nov 13, 2025
1 check passed

Conversation

kurapativyshnavi commented Oct 18, 2025

Uh oh!

rianrietveld commented Oct 20, 2025

Uh oh!

joedolson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rianrietveld left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rianrietveld Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

rianrietveld Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

joedolson Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

rianrietveld Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

joedolson Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rianrietveld commented Nov 10, 2025

Uh oh!

joedolson commented Nov 13, 2025

Uh oh!

rianrietveld left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Oct 23, 2025 •

edited

Loading

rianrietveld left a comment •

edited

Loading