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

Typeahed: allow search on focus #698

Closed
pkozlowski-opensource opened this issue Sep 7, 2016 · 14 comments
Closed

Typeahed: allow search on focus #698

pkozlowski-opensource opened this issue Sep 7, 2016 · 14 comments

Comments

@pkozlowski-opensource
Copy link
Member

In certain use-cases one might want to trigger typeahead search when the input field gets focused. You can see a good example of this on https://www.kayak.fr/flights

It should be trivial to implement this on our side since we should simply listen to the focus event, get whatever value we've got in the input and push it down the observable stream. The only tricky part might be taking into account the situation where a given input field has autofocus attribute (in this case we shouldn't kick off things).

There should be a flag for this option (I guess false by default).

I'm going to mark this as medium difficulty and I think that it is a good issue for ambitious community members. As always test are essentials.

@jnizet
Copy link
Member

jnizet commented Sep 11, 2016

Another problem to solve is that the usual distinctUntilChanged() operator, used to prevent the dropdown to open on two consecutive equal values, would prevent the window to open on focus events.

@pkozlowski-opensource
Copy link
Member Author

Another problem to solve is that the usual distinctUntilChanged() operator, used to prevent the dropdown to open on two consecutive equal values, would prevent the window to open on focus events.

Good point! We could un-subscribe on blur (actually - we must since there is a bug in the current implementation - see #723) and re-subscribe on focus.

@pkozlowski-opensource
Copy link
Member Author

This is blocked on #723

@Ks89
Copy link

Ks89 commented Mar 3, 2017

It could be really useful.
Which is the official scheduling for this and #723 ?

@GabrielGMartinsBr
Copy link

GabrielGMartinsBr commented Apr 20, 2017

So is it possible to implement or this is just a suggestion? Sorry for my dumb doubt, i'm trying to understand github yet

@beaverusiv
Copy link

I have a workaround for this for people who want this functionality now.

On the input element that has typeahead add a focus handler:

(focus)="onFocus($event)"

In the focus handler trigger an input event:

public onFocus(e: Event): void {
    e.stopPropagation();
    setTimeout(() => {
        const inputEvent: Event = new Event('input');
        e.target.dispatchEvent(inputEvent);
    }, 0);
}

It would be nice to not have to do this, but it works for now.

@Yura13
Copy link

Yura13 commented Jun 26, 2017

When will this issue be fixed?

@sscots
Copy link

sscots commented Aug 2, 2017

@beaverusiv, that works thanks!
However if I focus out and then focus back in it doesn't work the 2nd time.

@beaverusiv
Copy link

I have created a gist with the entire typeahead wrapper I have:

https://gist.github.com/beaverusiv/2d5818567b24bee200f5b4d6c333371c

Do with it as you please, but I will not provide support or answer questions about it. I believe @scotschroeder you will be interested mainly in ngAfterContentChecked()

@martinduris
Copy link

Hi guys,
is there any progress ? Or it is need to use workarounds like @beaverusiv suggesting ?

Thanks

@ymeine
Copy link
Contributor

ymeine commented Sep 15, 2017

Hello,

I worked on the issue to try figuring out solutions, ideas, and here they are.

The feature requirements

As seen by the end user:

  • be able to trigger a search on input focus so that the dropdown gets opened (or not if no result)

Additional requirements as seen by the application developer:

  • be able to opt-in/opt-out for this feature easily
  • be able to distinguish a search triggered by focus from one triggered by input change

An example of a scenario that should be possible with such a feature

  • application is initialized: input is empty, no input change has been made yet
  • I focus the input
    • ⇒ dropdown opens with all possible values
  • I type a few characters
    • ⇒ the list of result changes according to what I typed
  • I blur the input and close the dropdown
  • I focus back the input
    • ⇒ dropdown opens as it was just before closing it (i.e. with the same results corresponding to the same input value)
  • I remove all the characters by pressing backspace, so that the input gets empty, then two possible behaviors:
    • ⇒ I apply the general behavior, same as if the empty input just got focused: I display all possible values
    • ⇒ I decide that since I just intentionally removed the characters, it's a different case: I don't display anything

The last point illustrates that it is important to know whether the current search request comes from an input event or a focus event.

Possible implementations

Nothing

On the application side, it is already possible to add a focus event handler on the input. The listener can then send values to an internal Subject instance which would have been merged at some point with the observable given in the TypeAhead initialization function (ngbTypeahead).

Example usage:

import {Component} from '@angular/core';

import {Observable} from 'rxjs/Observable';
import {Subject} from 'rxjs/Subject'; // ★

import 'rxjs/add/operator/map';
import 'rxjs/add/operator/merge'; // ★
import 'rxjs/add/operator/debounceTime';
import 'rxjs/add/operator/distinctUntilChanged';

@Component({
  selector: 'app-root',
  template: `
  	<form>
		<input
			name='input'
			[(ngModel)]='searchTerm'
			[ngbTypeahead]='search'
			(focus)='handleFocus()' <!-- ★ -->

			type='text'
		/>
	</form>
	`
})
export class AppComponent {
	public searchTerm: string;

	private focus$ = new Subject<null>(); // ★

	handleFocus() {
		this.focus$.next(null); // ★
	}

	search = (input$: Observable<string>): Observable<any> => {
		return input$
		.debounceTime(200)
		.distinctUntilChanged()
		.merge(this.focus$) // ★
		.map(searchTerm => ...);
	}
}

Pros:

  • full control from the user

Cons:

  • some boilerplate code (creating the Subject, handling focus event)

Create ourselves an observable from the focus event and give it to the user

Example usage:

import {Component} from '@angular/core';

import {Observable} from 'rxjs/Observable';

import 'rxjs/add/operator/map';
import 'rxjs/add/operator/merge'; // ★
import 'rxjs/add/operator/debounceTime';
import 'rxjs/add/operator/distinctUntilChanged';

@Component({
  selector: 'app-root',
  template: `
  	<form>
		<input
			name='input'
			[(ngModel)]='searchTerm'
			[ngbTypeahead]='search'

			type='text'
		/>
	</form>
	`
})
export class AppComponent {
	public searchTerm: string;

	search = (input$: Observable<string>, /* ★ */ focus$: Observable<string>): Observable<any> => {
		return input$
		.debounceTime(200)
		.distinctUntilChanged()
		.merge(focus$) // ★
		.map(searchTerm => ...);
	}
}

Pros:

  • still full control from the user
  • backward compatible
  • easy to understand and manage
  • no boilerplate code

Cons: I don't see any. However it would require more API design, since I don't think we should extend the arguments list too much for any possible event. Also, we could optimize the implementation by providing an option so that if the feature is not wanted there is no new event handler added and no corresponding observable created.

Merge ourselves the focus observable with the input observable

There are two fashions for this one: backward compatible and non-backward compatible.

Backward compatible

Example usage:

//...
export class AppComponent {
	public searchTerm: string;

	search = (input$: Observable<string>): Observable<any> => {
		return input$
		.debounceTime(200)
		.distinctUntilChanged()
		.map(searchTerm => ...);
	}
}

Pros:

  • "works" out of the box
  • no boilerplate code

Cons:

  • NO WAY to distinguish focus events from input events, so implementing different behaviors is not possible
    • debounceTime is probably not wanted in case of focus
    • distinctUntilChanged could mess up in case of focus (depending on how we keep the rest of the implementation)

Non backward compatible

Example usage:

//...
export class AppComponent {
	public searchTerm: string;

	search = (input$: Observable<{from, value}> /* ★ */): Observable<any> => {
		return input$
		.debounceTime(200)
		.distinctUntilChanged()
		.map(payload => {
			const {from, value} = payload; // ★
			if (from === 'focus') {
				// ...
			} else {
				// ...
			}
		});
	}
}

Pros:

  • no boilerplate code

Cons:

  • hard for the user to process observables differently depending on if it comes from focus or input (again, for debounceTime and distinctUntilChanged), it requires a good knowledge of observables
  • not backward compatible since the given observable sends object values instead of just string values

Conclusion

I personally prefer the second option for these reasons:

  • backward compatible
  • simple to use on the application side
  • full control over the different events/observables
  • simple to implement on our side
  • we still provide something to avoid boilerplate code on the application side, while relying on common and powerful concepts (observables)

I will leave this pending for comments for some days, and if no decision stands out we will make one and implement it.

PS: this whole feature based on focus events requires a very small and simple fix inside the TypeAhead to avoid trying to write an undefined value inside the input element. With the methods exposed above, this can happen if no input event has been triggered yet, and the dropdown is open through a focus event and closed afterwards without selecting anything. Therefore using the first exposed implementation (fully on your side) won't work for that use case without this fix.

@beaverusiv
Copy link

I like the "Create ourselves an observable from the focus event and give it to the user" solution, I feel it gives the greatest flexibility for users and backward compatibility which to me is important to help keep libraries up to date and decoupled from when I may want to change the UX surrounding a component.

@ymeine
Copy link
Contributor

ymeine commented Sep 18, 2017

A few issues to be fixed for the implementation to work


I mentioned at the end of my previous comment that the implementation would require a small fix to be done regarding the management of the internal user input. This is not the only one, and the following I'm going to describe is due to the same problem: the value of the input element is written when the dropdown is closed using the internally stored _userInput property.

The problem comes from the fact that the latter property is changed only with input events, therefore being able to open the dropdown on other events (focus being the one we want here) changes the way things should be handled.

With the feature we want to implement, imagine the following scenario:

  • view loads, input is empty, _userInput is undefined
  • input is focused, dropdown opens, _userInput is still undefined
  • some text is written to filter options, _userInput contains the entered value as is (let's say it's incomplete)
  • option is clicked, dropdown closes, input value is written with the selected value, but _userInput remains with the entered value only
  • input is focused again, dropdown opens
  • dropdown is closed by clicking outside of the widget, _userInput is written in the input, but it doesn't contain the previously selected option value

This feature implementation would also require #1853 to be fixed to handle the case where the focus occurs because of a click in the input. Without it, the focus event would be received first and dropdown opened, and then the click event would be received and dropdown closed, making it impossible to open the dropdown immediately in this use case. Introducing a small delay for opening works but is not necessarily what we want.

@fredgate
Copy link

fredgate commented Nov 21, 2017

Take care that the hack of @beaverusiv can introduce another problems.

public onFocus(e: Event): void {
    e.stopPropagation();
    setTimeout(() => {
        const inputEvent: Event = new Event('input');
        e.target.dispatchEvent(inputEvent);
    }, 0);
}

Scenario :

  • enter some text in input
  • select an option in the dropdown. The value of the control is set.
  • go to another field and return to the field hosting the ngbTypeahead
  • the hack sends an input event that triggers dropdon to open.... but the value of the control is reset to undfined while no text has not been entered. If you leave the input doing anything else, the value of the control is undefined but the text in the HTML input did not changed.

This is very inconvenient to lost value just by giving focus to field without entering some text. And I do not see hack to eventually restore value corresponding to entered text when focus is lost. Any idea ?

So for future implemntation, we should be careful that value is not unset when field receives focus and dropdown is opened,while user enters no text.

May be a simple solution could be to provide a public open method that opens the dropdown whithout reseting value of control. We could then decide when to call this method in ours templates or components; for example on focus of input field, or click on a button (like a add-on button of bootsrap input group).

ymeine added a commit to ymeine/ng-bootstrap that referenced this issue Nov 24, 2017
ymeine added a commit to ymeine/ng-bootstrap that referenced this issue Dec 8, 2017
ymeine added a commit to ymeine/ng-bootstrap that referenced this issue Dec 11, 2017
ymeine added a commit to ymeine/ng-bootstrap that referenced this issue Dec 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests