alphagov / accessible-autocomplete

An autocomplete component, built to be accessible.
https://alphagov.github.io/accessible-autocomplete/examples/
MIT License
912 stars 149 forks source link

UA sniffing for iOS 10 #61

Open adamsilver opened 7 years ago

adamsilver commented 7 years ago

Here's the problem:

  1. User focuses on text box
  2. On-screen keyboard shows
  3. User types some letters
  4. Suggestions display beneath text box
  5. User hides the keyboard
  6. Suggestions hide before the user had a chance to select one by tapping one of them.

6 is the problem: when the user hides the on-screen keyboard, the blur event fires on the textbox, in iOS 10. I don't believe any other device does this including earlier versions of iOS.

I wonder if we can fix this by the approach we take to building this component.

Currently, the Javascript listens for the text box blur event and hides the suggestion list.

Here's a very early thought of mine...

Don't hide the suggestions on blur. Instead hide the suggestions when the user presses the tab key.

But what about clicking/tapping elsewhere on the page? In this case we should also hide the suggestions.

@tvararu raised concerns with listening to events that are outside of the components segment of the Document Tree.

I don't share this concern. This is the nature of developing Javascript components in the browser. It might also be that a component listens to load/scroll/resize events at the window/document/body level, for example.

As long as a component does not stop propagation/bubbling then this is, in my book, absolutely no problem.

Raising this here to open up the discussion. Critique welcomed.

tvararu commented 7 years ago

Thanks for raising this @adamsilver and for presenting both solutions. 👍

You are correct that the UA sniffing is there to prevent closing the typeahead menu when iOS 10 users close their on-screen keyboard, and that iOS 10 Safari is unique in this behaviour.

You are correct that an alternative solution is to not rely on the "blur" event, and instead capture every expected interaction that the user can perform that will move focus away from the typeahead. Effectively reimplementing "blur."

Here's how I think the two stack against each other.

UA sniffing

Pros:

Cons:

Reimplement blur

Pros:

Cons:


Happy to amend these with more input.

adamsilver commented 7 years ago

Breaks encapsulation by polluting the window object with additional event listeners (and no easy way to delegate for multiple instances)

What does this mean exactly?

tvararu commented 7 years ago

@adamsilver adding an event listener to the window object is breaking outside the boundaries of the component in order to mutate a global. There are various performance issues (that can be solved with event delegation) with doing it but I think they're not that important. The reason I raise this as a con is because it's a side effect.

When I use a component, I have the expectation that it will stick to its sandbox and not change other parts of my page or mutate my existing objects. Sticking to that contract gives me peace of mind that my components can't break one another as easily.

I agree that sometimes this isn't always avoidable as you eloquently phrased here:

This is the nature of developing Javascript components in the browser. It might also be that a component listens to load events/scroll/resize events at the window/document/body level, for example.

But in this situation, I think the better solution is to not do it. Let me know what you think.

adamsilver commented 7 years ago

@tvararu Thanks for that explanation.

Listening to an event on the window object is not "mutating a global".

I'm not meaning to be pedantic here. I only want to be clear about this, not least for my own understanding of your view point.

You go on to say that it might break things. Can you provide a code example of this?

In providing a code example, I can get a feel for your concern. At the moment it's a little bit theoretical and unclear to me what the actual problem is, or could be.

I'll start us off:

function TypeAhead() {
    window.addEventListener('click', function(e) { this.hideSuggestions() }.bind(this), false)
}

function OtherComponent() {
    window.addEventListener('click', function(e) { this.whatever() }.bind(), false)
}

new TypeAhead();
new OtherComponent();
// etc
tvararu commented 7 years ago

@adamsilver

Listening to an event on the window object is not "mutating a global".

But it is, the object being mutated is getEventListeners(window). 💃

That example you posted is inoffensive, but here's how it could break things with one more line:

 function TypeAhead() {
     window.addEventListener('click', function (e) {
+      e.preventDefault()
       this.hideSuggestions()
     }.bind(this), false)
 }

Additionally, now you need to handle this:

var t = new Typeahead()
delete t
// The event listener is still attached to the window.

And this:

var arr = Array(1000).fill()
  .map(() => new Typeahead())
// You now have 1000 click event listeners on window that do the same thing.

I hope that helps; the examples above are ugly enough to me to want to avoid relying on global event listeners when building an isolated component.

adamsilver commented 7 years ago

But it is, the object being mutated is getEventListeners(window). 💃

Firstly, nice dance moves.

Secondly I don't see that as the same thing as messing with mutating a global (host object). But I guess you've proven that to be an opinion somewhat :)

Here's some references to what I was referring to:

Additionally, now you need to handle this: var t = new Typeahead() delete t // The event listener is still attached to the window.

You need to handle that anyway. And to handle it you would need a TypeAhead.prototype.destroy method.

And this: var arr = Array(1000).fill() .map(() => new Typeahead()) // You now have 1000 click event listeners on window that do the same thing.

  1. Is this actually a performance problem?
  2. Will a UI really have 1000 different listeners on the same event?
  3. It's quite easy to solve. Here's what I have done in the past (half pseudo code):

    
    var events = {
      windowClicked: new CustomEvent()
    };
    
    // some GOVUK window component thinger
    window.addEventListener('click', function(e) {
      events.windowClicked.fire(e);
    }});
    
    function TypeAhead() {
      events.windowClicked.on(this.handleWindowClick.bind(this));
    }
    
    TypeAhead.prototype.onWindowClick = function(e) {
      // do stuff
    };
    
    TypeAhead.prototype.destroy = function(e) {
      events.windowClicked.remove(...);
    };

I hope this helps.

NickColley commented 7 years ago

It seems like if we're careful we can indeed have global listeners, we could expose it to the component in a way that users would not be able to preventDefault etc as Adam has noted, so that leaves

More difficult to implement (tab, click, what are all the ways a user can blur something?)

Are we confident that what we'd be listening for would work across browsers, would be good to explore that more, if there's a future friendly way of doing that it seems like a better solution than sniffing?

adamsilver commented 7 years ago

I actually forgot to address the bit about preventing the default (e.preventDefault).

I don't see this as an issue anyway. Anyone can do "stupid" things in code. And even with my example I have exposed the event object, allowing them to do the same thing.

We don't have to expose the event object but that is an overly protective way to go, to the extent developers who need to get at the event will find it difficult/tedious to do so.

NickColley commented 7 years ago

In this case, only the maintainers of the component will have access to those listeners anyway since it's not going to be a publicly exposed API, so seems fine.

Will see what @tvararu thinks, but I think while there are cons for global listeners, they out weigh sniffing if we can ensure the events we're listening on are future friendly...

tvararu commented 7 years ago

@adamsilver okay, you're right that there are ways to work around the problems with global event listeners, and perhaps I blew them a bit out of proportion. In the end my gripe is that it's an ugly pattern.

I've realised that it may be possible that we have no choice but to reimplement blur; these two issues are also related to the behaviours around the event: https://github.com/alphagov/accessible-typeahead/issues/48, https://github.com/alphagov/accessible-typeahead/issues/64.

So yes, I agree, let's explore what an implementation using this approach would look like. Maybe we can kill 3 birds with one stone.

tvararu commented 7 years ago

Food for thought about this from this article:

Typically an editable Combobox input element such as this will generate a list of associated auto-suggest options, which will disappear when the focus moves away from the edit field. This is done using onBlur on the edit field, however this presents critical accessibility issues on touch screen devices. When VoiceOver is running on iOS for example, simply moving one finger around the screen to hear what information the screen contains including what is listed in the associated auto-suggest widget will automatically fire all onFocus and onBlur handlers that are encountered at the same time causing the associated widget to close automatically, thus making it impossible to read the suggested list using VoiceOver.

To solve this, logic must be added to the onBlur script so that the auto-suggest widget will only close automatically when a touch screen device is not being used, otherwise the associated widget will need an additional Close button that can be tapped to close it if desired.

So an additional approach to fix this is to instead feature detect if the device has a touch screen.

I don't think that's a great approach because:

adamsilver commented 7 years ago

I have spent some more time thinking about these issues. And I've also spoken to David Mark (@cinsoft) who is really good at this.

If iOS 10 (for example) is doing some silly things—that is, if it fires the blur event when it shouldn't—then so be it. I would hope/expect iOS fixes this in upcoming releases.

The user is still able to type. If they enter something invalid we can handle this through submission:

1) If, for example, I type "Englan" and submit, the server could either process that as "England" or...

2) if, there are many matches, we could present an error message, inviting the user to fix. As part of the error message, or in addition to the error message we could facilitate the correction process:

Choose Country You typed "Englan". We found the following suggestions: (RADIO BUTTON) England (RADIO BUTTON) Etc [AUTOCOMPLETE TEXT BOX HERE READY FOR USER TO TYPE AGAIN IF THEY WANT]

(Or something like that)

This is what progressive enhancement is about. We enhance the experience where possible. We degrade where it isn't.

tvararu commented 7 years ago

@adamsilver That's a solution.

But:

The default blur + combobox behaviour is broken on iOS, as mentioned in the article I linked to above. A typeahead that does not work around it is not being as accessible as it can.

Let's say the user wants to select "The United Mexican States" in this example:

img_2347

They can just barely make out the name at the bottom of the list, but if they decide to close the keyboard, the blur event will fire and the suggestions will vanish. This is frustrating and unexpected. The user is unlikely to submit the form regardless; they're likely to give up.

adamsilver commented 7 years ago

I totally understand that the experience I suggested is far from ideal. Your explanation perfectly demonstrates this.

It's just that the work arounds are also far from ideal.

It's a difficult trade either way. At the very least, I wanted to offer up an alternative.

Has a bug been raised for iOS10?

adamsilver commented 7 years ago

According to spec we should also provide a button to open the list:

The next element should be an html

This is something Ed and I discussed in issue 31.

If we provide an explicit button, the user can always reveal the options without having to type. I think this is a good feature in it's own right, but it may also help with regard to the iOS10 issue.

tvararu commented 7 years ago

@adamsilver It think would be interesting to try out the button pattern.

Has a bug been raised for iOS10?

Probably not, I haven't at least.

It's a difficult trade either way. At the very least, I wanted to offer up an alternative.

Agree, and your input has been and is valuable! 👍