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

Implemented FocusCycler#forwardCycle and #backwardCycle events #15050

Merged
merged 7 commits into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
98 changes: 82 additions & 16 deletions packages/ckeditor5-ui/src/focuscycler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import {
isVisible,
type ArrayOrItem,
type FocusTracker,
type KeystrokeHandler
type KeystrokeHandler,
EmitterMixin
} from '@ckeditor/ckeditor5-utils';

import type View from './view';
Expand Down Expand Up @@ -69,7 +70,7 @@ import type ViewCollection from './viewcollection';
*
* Check out the {@glink framework/deep-dive/ui/focus-tracking "Deep dive into focus tracking"} guide to learn more.
*/
export default class FocusCycler {
export default class FocusCycler extends EmitterMixin() {
/**
* A {@link module:ui/view~View view} collection that the cycler operates on.
*/
Expand Down Expand Up @@ -116,6 +117,8 @@ export default class FocusCycler {
keystrokeHandler?: KeystrokeHandler;
actions?: FocusCyclerActions;
} ) {
super();

this.focusables = options.focusables;
this.focusTracker = options.focusTracker;
this.keystrokeHandler = options.keystrokeHandler;
Expand Down Expand Up @@ -146,7 +149,7 @@ export default class FocusCycler {
* **Note**: Hidden views (e.g. with `display: none`) are ignored.
*/
public get first(): FocusableView | null {
return ( this.focusables.find( isFocusable ) || null ) as FocusableView | null;
return ( this.focusables.find( isPlainFocusableView ) || null ) as FocusableView | null;
}

/**
Expand All @@ -156,7 +159,7 @@ export default class FocusCycler {
* **Note**: Hidden views (e.g. with `display: none`) are ignored.
*/
public get last(): FocusableView | null {
return ( this.focusables.filter( isFocusable ).slice( -1 )[ 0 ] || null ) as FocusableView | null;
return ( this.focusables.filter( isPlainFocusableView ).slice( -1 )[ 0 ] || null ) as FocusableView | null;
}

/**
Expand Down Expand Up @@ -210,7 +213,7 @@ export default class FocusCycler {
* **Note**: Hidden views (e.g. with `display: none`) are ignored.
*/
public focusFirst(): void {
this._focus( this.first );
this._focus( this.first, 1 );
}

/**
Expand All @@ -219,7 +222,7 @@ export default class FocusCycler {
* **Note**: Hidden views (e.g. with `display: none`) are ignored.
*/
public focusLast(): void {
this._focus( this.last );
this._focus( this.last, -1 );
}

/**
Expand All @@ -228,7 +231,13 @@ export default class FocusCycler {
* **Note**: Hidden views (e.g. with `display: none`) are ignored.
*/
public focusNext(): void {
this._focus( this.next );
const next = this.next;

this._focus( next, 1 );

if ( next == this.first ) {
this.fire<FocusCyclerForwardCycleEvent>( 'forwardCycle' );
scofalik marked this conversation as resolved.
Show resolved Hide resolved
}
}

/**
Expand All @@ -237,14 +246,34 @@ export default class FocusCycler {
* **Note**: Hidden views (e.g. with `display: none`) are ignored.
*/
public focusPrevious(): void {
this._focus( this.previous );
const previous = this.previous;

this._focus( previous, -1 );

if ( previous == this.last ) {
this.fire<FocusCyclerBackwardCycleEvent>( 'backwardCycle' );
}
}

/**
* Focuses the given view if it exists.
*
* @param view The view to be focused
* @param childrenFocusDirection The direction of the focus if the view has focusable children.
* @returns
*/
private _focus( view: FocusableView | null ) {
if ( view ) {
private _focus( view: FocusableView | null, childrenFocusDirection: 1 | -1 ) {
if ( !view ) {
return;
}

if ( isViewWithFocusableChildren( view ) ) {
if ( childrenFocusDirection === 1 ) {
view.focusFirst();
} else if ( childrenFocusDirection === -1 ) {
view.focusLast();
}
} else {
Copy link
Contributor

@scofalik scofalik Sep 24, 2023

Choose a reason for hiding this comment

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

What if I have a view that can be focused AND has focusable children? Shouldn't we let the view decide what it does upon being focused? EDIT: Actually, this is the definition of ViewWithFocusableChildren :D. So, that view is in theory focusable but it will never actually get the focus because it will always focus its child.

I am not sure if this logic should be inside FocusCycler but this is your domain so you decide.

view.focus();
}
}
Expand Down Expand Up @@ -276,7 +305,7 @@ export default class FocusCycler {
do {
const view = this.focusables.get( index )!;

if ( isFocusable( view ) ) {
if ( isPlainFocusableView( view ) ) {
return view as FocusableView;
}

Expand All @@ -289,11 +318,19 @@ export default class FocusCycler {
}

/**
* A {@link module:ui/view~View} that can be focused, meaning it has a `focus()` method.
* A view that can be focused.
*/
export interface FocusableView extends View {
export type FocusableView = View & {
focus(): void;
}
};

/**
* A view that can be focused and contains some children that also can be focused.
*/
export type ViewWithFocusableChildren = FocusableView & {
focusFirst(): void;
focusLast(): void;
};

export interface FocusCyclerActions {
focusFirst?: ArrayOrItem<string>;
Expand All @@ -302,11 +339,40 @@ export interface FocusCyclerActions {
focusPrevious?: ArrayOrItem<string>;
}

/**
* Fired when the focus cycler is about to move the focus from the last focusable item
* to the first one.
*
* @eventName ~FocusCycler#forwardCycle
*/
export type FocusCyclerForwardCycleEvent = {
name: 'forwardCycle';
args: [];
};

/**
* Fired when the focus cycler is about to move the focus from the first focusable item
* to the last one.
*
* @eventName ~FocusCycler#backwardCycle
*/
export type FocusCyclerBackwardCycleEvent = {
name: 'backwardCycle';
args: [];
};

/**
* Checks whether a view is focusable.
*
* @param view A view to be checked.
*/
function isFocusable( view: View & { focus?: unknown } ) {
return !!( view.focus && isVisible( view.element ) );
function isPlainFocusableView( view: View ): view is FocusableView {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is a "plain focusable view"? AFAICS, the implementation did not change. If this is to differentiate "focusable" from "focusable with focusable children", I wouldn't make a change. To me, it felt like maybe "plain focsuable view" is a focusable that is not a "focusable with focusable children". But that's not true. I might not get something, but at the first glance, the change and the new name is confusing.

return !!( 'focus' in view && isVisible( view.element ) );
}

/**
* Checks whether the view meets the requirements of the {@link ViewWithFocusableChildren} interface.
*/
function isViewWithFocusableChildren( view: View ): view is ViewWithFocusableChildren {
return isPlainFocusableView( view ) && 'focusFirst' in view && 'focusLast' in view;
}
8 changes: 7 additions & 1 deletion packages/ckeditor5-ui/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,13 @@ export { default as BoxedEditorUIView } from './editorui/boxed/boxededitoruiview
export { default as InlineEditableUIView } from './editableui/inline/inlineeditableuiview';

export { default as FormHeaderView } from './formheader/formheaderview';
export { default as FocusCycler, type FocusableView } from './focuscycler';
export {
default as FocusCycler,
type FocusableView,
type ViewWithFocusableChildren,
type FocusCyclerForwardCycleEvent,
type FocusCyclerBackwardCycleEvent
} from './focuscycler';

export { default as IconView } from './icon/iconview';
export { default as InputView } from './input/inputview';
Expand Down
7 changes: 7 additions & 0 deletions packages/ckeditor5-ui/src/list/listview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,13 @@ export default class ListView extends View<HTMLUListElement> implements Dropdown
this._focusCycler.focusFirst();
}

/**
* Focuses the first focusable in {@link #items}.
*/
public focusFirst(): void {
this._focusCycler.focusFirst();
}

/**
* Focuses the last focusable in {@link #items}.
*/
Expand Down
5 changes: 2 additions & 3 deletions packages/ckeditor5-ui/src/search/filteredview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

import type { FocusableView } from '../focuscycler';
import type View from '../view';
import type { ViewWithFocusableChildren } from '../focuscycler';

/**
* @module ui/search/filteredview
Expand All @@ -13,7 +12,7 @@ import type View from '../view';
/**
* A view that can be filtered by a {@link module:ui/search/text/searchtextview~SearchTextView}.
*/
export default interface FilteredView extends View, FocusableView {
export default interface FilteredView extends ViewWithFocusableChildren {

/**
* Filters the view by the given regular expression.
Expand Down
13 changes: 11 additions & 2 deletions packages/ckeditor5-ui/src/search/searchinfoview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@
* @module ui/search/searchinfoview
*/

import type { FocusableView } from '../focuscycler';
import View from '../view';

/**
* A view displaying an information text related to different states of {@link module:ui/search/text/searchtextview~SearchTextView}.
*
* @internal
*/
export default class SearchInfoView extends View {
export default class SearchInfoView extends View implements FocusableView {
/**
* Controls whether the view is visible.
*
Expand Down Expand Up @@ -60,7 +61,8 @@ export default class SearchInfoView extends View {
'ck',
'ck-search__info',
bind.if( 'isVisible', 'ck-hidden', value => !value )
]
],
tabindex: -1
},
children: [
{
Expand All @@ -82,4 +84,11 @@ export default class SearchInfoView extends View {
]
} );
}

/**
* Focuses the view
*/
public focus(): void {
this.element!.focus();
}
}
57 changes: 49 additions & 8 deletions packages/ckeditor5-ui/src/search/searchresultsview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,42 @@

import View from '../view';
import type ViewCollection from '../viewcollection';
import type { Locale } from '@ckeditor/ckeditor5-utils';
import type { FocusableView } from '../focuscycler';
import { FocusTracker, type Locale } from '@ckeditor/ckeditor5-utils';
import { default as FocusCycler, type ViewWithFocusableChildren } from '../focuscycler';

/**
* A sub-component of {@link module:ui/search/text/searchtextview~SearchTextView}. It hosts the filtered and the information views.
*/
export default class SearchResultsView extends View implements FocusableView {
export default class SearchResultsView extends View implements ViewWithFocusableChildren {
/**
* Tracks information about the DOM focus in the view.
*
* @readonly
*/
public focusTracker: FocusTracker;

/**
* The collection of the child views inside of the list item {@link #element}.
*
* @readonly
*/
public children: ViewCollection;

/**
* Provides the focus management (keyboard navigation) in the view.
*
* @readonly
*/
protected _focusCycler: FocusCycler;

/**
* @inheritDoc
*/
constructor( locale: Locale ) {
super( locale );

this.children = this.createCollection();
this.focusTracker = new FocusTracker();

this.setTemplate( {
tag: 'div',
Expand All @@ -42,16 +57,42 @@ export default class SearchResultsView extends View implements FocusableView {
},
children: this.children
} );

this._focusCycler = new FocusCycler( {
focusables: this.children,
focusTracker: this.focusTracker
} );
}

/**
* Focuses the first child view.
* @inheritDoc
*/
public focus(): void {
const firstFocusableChild = this.children.find( ( child: any ) => typeof child.focus === 'function' );
public override render(): void {
super.render();

if ( firstFocusableChild ) {
( firstFocusableChild as FocusableView ).focus();
for ( const child of this.children ) {
this.focusTracker.add( child.element! );
}
}

/**
* Focuses the view.
*/
public focus(): void {
this._focusCycler.focusFirst();
}

/**
* Focuses the first child view.
*/
public focusFirst(): void {
this._focusCycler.focusFirst();
}

/**
* Focuses the last child view.
*/
public focusLast(): void {
this._focusCycler.focusLast();
}
}
2 changes: 1 addition & 1 deletion packages/ckeditor5-ui/src/search/text/searchtextview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ export default class SearchTextView<
*
* @readonly
*/
private _focusCycler: FocusCycler;
protected _focusCycler: FocusCycler;

/**
* The cached configuration object.
Expand Down
Loading