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

LinkControl - adds search results label for initial suggestions #19665

Merged
merged 6 commits into from
Jan 15, 2020
+32 −6
Merged

LinkControl - adds search results label for initial suggestions #19665

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
febda74
Adds search results label for initial suggestions
getdave Jan 15, 2020
4eaee3d
Conditional render visually hidden label for normal search results
getdave Jan 15, 2020
2f98d96
Tweak wording from “modified” to “updated”
getdave Jan 15, 2020
ff77275
Fixes unecessary boolean inversion.
getdave Jan 15, 2020
f6ee1c6
Refactor arial-labelled by and aria-label implementation to confirm t…
getdave Jan 15, 2020
8e0dc91
Use undefined instead of boolean false to avoid rendering of id and a…
getdave Jan 15, 2020
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
14 changes: 11 additions & 3 deletions packages/block-editor/src/components/link-control/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { noop, startsWith } from 'lodash';
* WordPress dependencies
*/
import { Button, ExternalLink } from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { __, sprintf } from '@wordpress/i18n';
import { useCallback, useState, Fragment } from '@wordpress/element';

import {
Expand Down Expand Up @@ -131,16 +131,24 @@ export function LinkControl( {
}, [ handleDirectEntry, fetchSearchSuggestions ] );

// Render Components
const renderSearchResults = ( { suggestionsListProps, buildSuggestionItemProps, suggestions, selectedSuggestion, isLoading } ) => {
const renderSearchResults = ( { suggestionsListProps, buildSuggestionItemProps, suggestions, selectedSuggestion, isLoading, isInitialSuggestions } ) => {
const resultsListClasses = classnames( 'block-editor-link-control__search-results', {
'is-loading': isLoading,
} );

const manualLinkEntryTypes = [ 'url', 'mailto', 'tel', 'internal' ];
const searchResultsLabelId = `block-editor-link-control-search-results-label-${ instanceId }`;
const labelText = isInitialSuggestions ? __( 'Recently modified' ) : sprintf( __( 'Search results for %s' ), inputValue );

return (
<div className="block-editor-link-control__search-results-wrapper">
<div { ...suggestionsListProps } className={ resultsListClasses }>
{ isInitialSuggestions && (
<span className="block-editor-link-control__search-results-label" id={ searchResultsLabelId }>
{ labelText }
</span>
) }

<div { ...suggestionsListProps } className={ resultsListClasses } aria-labelledby={ searchResultsLabelId }>
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice job on adding aria-labelledby! 🙌

I think there may be a slight difference on using that on visible vs hidden labels: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/listbox_role

I'm not positive I'm fully understanding the implementation though:

aria-label
A human-readable string value which identifies the listbox. If there's a visible label, then aria-labelledby should be used instead to refer to that label.

aria-labelledby
Identifies the visible element or elements in a space-separated list of element IDs which identify the listbox. If there's no visible label, then aria-label should be used instead to include a label. (Note: "labelled", with two L's, is the correct spelling based on the accessibility API conventions.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch but I don't follow the a11y guidelines here. Do I need to just add aria-label on the element when it's not visible?

{ suggestions.map( ( suggestion, index ) => (
<LinkControlSearchItem
key={ `${ suggestion.id }-${ suggestion.type }` }
Expand Down
6 changes: 6 additions & 0 deletions packages/block-editor/src/components/link-control/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@
}
}

.block-editor-link-control__search-results-label {
padding: 15px 30px 0 30px;
display: block;
font-size: 1.1em;
getdave marked this conversation as resolved.
Show resolved Hide resolved
}

.block-editor-link-control__search-results {
margin: 0;
padding: $grid-size-large/2 $grid-size-large $grid-size-large;
Expand Down
7 changes: 6 additions & 1 deletion packages/block-editor/src/components/link-control/test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,10 @@ describe( 'Default search suggestions', () => {
const searchInput = container.querySelector( 'input[aria-label="URL"]' );

// TODO: select these by aria relationship to autocomplete rather than arbitary selector.
const initialSearchResultElements = container.querySelectorAll( '[role="listbox"] [role="option"]' );
const searchResultsWrapper = container.querySelector( '[role="listbox"]' );
const initialSearchResultElements = searchResultsWrapper.querySelectorAll( '[role="option"]' );

const searchResultsLabel = container.querySelector( `#${ searchResultsWrapper.getAttribute( 'aria-labelledby' ) }` );
getdave marked this conversation as resolved.
Show resolved Hide resolved

// Verify input has no value has default suggestions should only show
// when this does not have a value
Expand All @@ -342,6 +345,8 @@ describe( 'Default search suggestions', () => {

// Verify the search results already display the initial suggestions
expect( initialSearchResultElements ).toHaveLength( expectedResultsLength );

expect( searchResultsLabel.innerHTML ).toEqual( 'Recently modified' );
} );

it( 'should not display initial suggestions when input value is present', async () => {
Expand Down
2 changes: 2 additions & 0 deletions packages/block-editor/src/components/url-input/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ class URLInput extends Component {
placeholder = __( 'Paste URL or type to search' ),
value = '',
autoFocus = true,
__experimentalShowInitialSuggestions = false,
} = this.props;

const {
Expand Down Expand Up @@ -365,6 +366,7 @@ class URLInput extends Component {
buildSuggestionItemProps,
isLoading: loading,
handleSuggestionClick: this.handleOnClick,
isInitialSuggestions: __experimentalShowInitialSuggestions && ! ( value && value.length ),
} ) }

{ ! isFunction( renderSuggestions ) && showSuggestions && !! suggestions.length &&
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/navigation-link/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ function NavigationLinkEdit( {
<LinkControl
className="wp-block-navigation-link__inline-link-input"
value={ link }
initialSuggestions={ true }
showInitialSuggestions={ true }
onChange={ ( {
title: newTitle = '',
url: newURL = '',
Expand Down