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.

Sign up for GitHub

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

Change search to only show results after submit #263

Merged
merged 13 commits into from
Oct 6, 2021
Merged

Change search to only show results after submit #263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
4989dd8
Add optional search page
lfdebrux Sep 21, 2021
7818e92
Change search to only show results on search page
lfdebrux Sep 21, 2021
e8c775d
Change search results to be normal flow page content
lfdebrux Sep 21, 2021
35a320e
Change search input to be always visible on search page for mobile
lfdebrux Sep 21, 2021
cc7f023
Add id to heading on search page
lfdebrux Sep 21, 2021
9524fa7
Fix bug where search does not work for URL path /search/
lfdebrux Sep 23, 2021
2bd83a9
Remove role alert from search results heading
lfdebrux Sep 30, 2021
c7b7402
Add JavaScript to update page title for search page
lfdebrux Sep 30, 2021
12eae2f
Change search results heading to h1
lfdebrux Sep 30, 2021
56e0b15
Add search query to search page heading
lfdebrux Sep 30, 2021
17a16bf
Update content for search help
lfdebrux Oct 1, 2021
fdf273b
Change JavaScript to only download search index on search page
lfdebrux Oct 6, 2021
ea5f829
Update CHANGELOG.md
lfdebrux Sep 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@

## Unreleased

### Breaking changes

The search user experience is now more accessible for screenreader users. Search results are now on a separate page instead of a modal window.

Users can no longer see search results as they type. They must press the Return key or select the search button to see the search results page.

This was added in [pull request #263: Change search to only show results after submit](https://github.com/alphagov/tech-docs-gem/pull/263).

### Fixes

- [#265: Fix mark styles in Windows High Contrast Mode](https://github.com/alphagov/tech-docs-gem/pull/265)

## 2.4.3
Expand Down
125 changes: 59 additions & 66 deletions lib/assets/javascripts/_modules/search.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,35 +6,48 @@
Modules.Search = function Search () {
var s = this
var $html = $('html')
var $tocNav
var $searchForm
var $searchLabel
var $searchInput
var $searchResults
var $searchResultsTitle
var $searchResultsWrapper
var $searchResultsClose
var $searchHelp
var results
var query
var queryTimer
var maxSearchEntries = 20

this.start = function start ($element) {
$searchForm = $element.find('form')
$searchInput = $element.find('#search')
$searchLabel = $element.find('.search__label')
$searchResultsWrapper = $element.find('.search-results')
$searchResultsWrapper = $('#search-results')
$searchResults = $searchResultsWrapper.find('.search-results__content')
$searchResultsTitle = $searchResultsWrapper.find('.search-results__title')
$searchResultsClose = $searchResultsWrapper.find('.search-results__close')
$tocNav = $('#toc')
s.downloadSearchIndex()
attach()
$searchHelp = $('#search-help')

changeSearchAction()
changeSearchLabel()

// Only do searches on the search page
if (s.isOnSearchPage()) {
s.downloadSearchIndex()
$html.addClass('has-search-results-open')

if (window.location.search) {
query = s.getQuery()
if (query) {
$searchInput.val(query)
doSearch(query)
doAnalytics()
document.title = query + ' - ' + document.title
}
}
}
}

this.downloadSearchIndex = function downloadSearchIndex () {
updateTitle('Loading search index')
updateTitle('Loading search results')
$.ajax({
url: '/search.json',
cache: true,
Expand All @@ -48,46 +61,45 @@
})
}

function attach () {
// Search functionality on search text input
$searchInput.on('input', function (e) {
e.preventDefault()
query = $(this).val()
s.search(query, function (r) {
results = r
renderResults(query)
updateTitle()
})
if (window.ga) {
window.clearTimeout(queryTimer)
queryTimer = window.setTimeout(sendQueryToAnalytics, 1000)
}
})
function changeSearchAction () {
// We need JavaScript to do search, so if JS is not available the search
// input sends the query string to Google. This JS function changes the
// input to instead send it to the search page.
$searchForm.prop('action', '/search')
$searchForm.find('input[name="as_sitesearch"]').remove()
}

// Set focus on the first search result instead of submiting the search
// form to Google
$searchForm.on('submit', function (e) {
e.preventDefault()
showResults()
var $firstResult = $searchResults.find('.search-result__title a').first()
if ($firstResult.length) {
$firstResult.focus()
} else {
// if there are no results, show the "0 results" state
results = []
updateTitle()
}
})
function changeSearchLabel () {
$searchLabel.text('Search this documentation')
}

// Closing the search results, move focus back to the search input
$searchResultsClose.on('click', function (e) {
e.preventDefault()
$searchInput.focus()
hideResults()
this.isOnSearchPage = function isOnSearchPage () {
return Boolean(window.location.pathname.match(/\/search(\/|\/index.html)?$/))
}

this.getQuery = function getQuery () {
var query = decodeURIComponent(
window.location.search
.match(/q=([^&]*)/)[1]
.replace(/\+/g, ' ')
)
return query
}

function doSearch (query) {
s.search(query, function (r) {
results = r
renderResults(query)
updateTitle()
})
}

// Attach analytics events to search result clicks
// TODO: remove this and sendQueryToAnalytics in a future breaking release
function doAnalytics () {
if (window.ga) {
sendQueryToAnalytics()

// Attach analytics events to search result clicks
$searchResults.on('click', '.search-result__title a', function () {
var href = $(this).attr('href')
ga('send', {
Expand All @@ -99,15 +111,6 @@
})
})
}

// When selecting navigation link, close the search results.
$('#toc').on('click', 'a', function (e) {
hideResults()
})
}

function changeSearchLabel () {
$searchLabel.text('Search this documentation')
}

function getResults (query) {
Expand All @@ -122,7 +125,6 @@

this.search = function search (query, callback) {
if (query === '') {
hideResults()
return
}
showResults()
Expand Down Expand Up @@ -192,23 +194,14 @@
function updateTitle (text) {
if (typeof text === 'undefined') {
var count = results.length
var resultsText = count + ' results'
var resultsText = 'Search - ' + query + ' - ' + count + ' results'
}
$searchResultsTitle.text(text || resultsText)
}

function showResults () {
$searchResultsWrapper.addClass('is-open')
.attr('aria-hidden', 'false')
$html.addClass('has-search-results-open')
$tocNav.addClass('search-results-open')
}

function hideResults () {
$searchResultsWrapper.removeClass('is-open')
.attr('aria-hidden', 'true')
$html.removeClass('has-search-results-open')
$tocNav.removeClass('search-results-open')
$searchResultsWrapper.removeAttr('hidden')
$searchHelp.attr('hidden', 'true')
ChrisBAshton marked this conversation as resolved.
Show resolved Hide resolved
}

function sendQueryToAnalytics () {
Expand Down
1 change: 0 additions & 1 deletion lib/assets/javascripts/_modules/table-of-contents.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@

function closeNavigation () {
$html.removeClass('toc-open')
$html.removeClass('has-search-results-open')

toggleBackgroundVisiblity(true)
updateAriaAttributes()
Expand Down
108 changes: 35 additions & 73 deletions lib/assets/stylesheets/modules/_search.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,48 +52,44 @@ $input-size: 40px;
}
}

html.has-search-results-open {
overflow: hidden;
.app-pane__content {
overflow: hidden;
}

.toc__close{
display: none;
@include govuk-media-query($until: tablet) {
html.js.has-search-results-open:not(.toc-open) {
.toc {
display: block;
padding-bottom: 0;
}

.toc__close {
display: none;
}

.toc__list {
display: none;
}
}
}

.search-results {
display: none;
&.is-open {
display: block;
@include govuk-media-query(tablet) {
// Create basis for the search results caret (below)
position: relative;

@include govuk-font($size: 16);
padding-top: govuk-spacing(6);
}
}

.search-results {
position: absolute;
top: 60px;
left: 0;
right: 0;
bottom: 0;
z-index: 600;
overflow-x: scroll;
-webkit-overflow-scrolling: touch;
-ms-overflow-style: none;
@include govuk-media-query(tablet) {
padding: govuk-spacing(6);
top: 0;
// The width of the sidebar
left: 330px;
min-height: auto;
}
a {
@include govuk-link-common;
@include govuk-link-style-no-visited-state;
}

a {
@include govuk-link-common;
@include govuk-link-style-no-visited-state;
}
ul {
list-style: none;
padding: 0;
margin: 0;
}
}
.search-results__inner {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove some of the padding in this class, which presumably was only in place to allow space for the Close button, which no longer exists. See screenshot, where the right-hand padding (in DevTools view, so smaller screen) means little space for the results text.

Screenshot 2021-10-05 at 10 55 08

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I see your point, but I'm reluctant to make a change without input for a designer... I'd want to make sure any change would be making things better rather than worse! I think it's important to keep a certain amount of right padding, but that amount might be different at different viewports.

I think the best thing for now would be to create a new issue about this, is that something you would be happy to do? Sorry to be difficult!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally understood. I'll raise an issue after this PR is merged (as it isn't an issue until then!).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've now raised #269.

position: relative;
background: govuk-colour("white");
border-top: 1px solid govuk-colour("mid-grey");
max-width: 40rem;
Expand All @@ -104,7 +100,7 @@ html.has-search-results-open {
&::after {
content: '';
position: absolute;
top: 10px;
top: 35px;
left: -9px;
width: 10px;
height: 20px;
Expand All @@ -117,44 +113,6 @@ html.has-search-results-open {
@include govuk-font($size: 27, $weight: bold);
margin-bottom: govuk-spacing(6);
}
.search-results__close {
@include govuk-font($size: 16);
position: absolute;
top: 18px;
right: 20px;
appearance: none;
-webkit-appearance: none;
background: none;
border: 0;
padding: 0;
cursor: pointer;

&:focus {
@include govuk-focused-text;
}

&::after {
content: '';
display: inline-block;
vertical-align: middle;
padding-left: 8px;
height: 18px;
width: 18px;
background: no-repeat url('/images/govuk-icn-close.png') center right;
@include govuk-device-pixel-ratio {
background-image: url('/images/[email protected]');
}
background-size: contain;
}
}
.search-results__close-label {
position: absolute;
left: -9999em;
top: auto;
width: 1px;
height: 1px;
overflow: hidden;
}
.search-result {
margin-bottom: govuk-spacing(6);
}
Expand All @@ -179,3 +137,7 @@ html.has-search-results-open {
color: CanvasText;
}
}

.js .search-help__no-js {
display: none;
}
4 changes: 0 additions & 4 deletions lib/assets/stylesheets/modules/_technical-documentation.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@
margin: 0 govuk-spacing(6) govuk-spacing(6);
}

.has-search-results-open & {
visibility: hidden;
}

> h1 {
@extend %govuk-heading-xl;

Expand Down
3 changes: 0 additions & 3 deletions lib/assets/stylesheets/modules/_toc.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,6 @@

.toc__list {
margin-right: govuk-spacing(7);
&.search-results-open {
display: none;
}
}

.toc__close {
Expand Down
2 changes: 2 additions & 0 deletions lib/govuk_tech_docs.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ def active_page(page_path)

search.tokenizer_separator = '/[\s\-/]+/'
end
else
context.ignore "search/*"
end
end
end
7 changes: 0 additions & 7 deletions lib/source/layouts/_search.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,5 @@
placeholder="Search">
<button type="submit" class="search__button">Search</button>
</form>
<div id="search-results" class="search-results" aria-hidden="true" role="dialog" aria-labelledby="search-results-title">
<div class="search-results__inner">
<button class="search-results__close">Close<span class="search-results__close-label"> search results</span></button>
<h2 id="search-results-title" class="search-results__title" aria-live="assertive" role="alert">Results</h2>
<div class="search-results__content"></div>
</div>
</div>
</div>
<% end %>
Loading