Skip to content

Conversation

@e-hein
Copy link
Contributor

@e-hein e-hein commented Jul 11, 2020

Currently arrow keys have no effect in TestbedEnvironment and sending keys does not respect cursor position or selections.

This adjusts behavior in TestbedEnvironment to get closer to the behavior of ProtractorEnvironment (real browsers).

It's a prerequisite to #19709 (will need select on tab into input with text). But in fact this is the larger part because selection handling is much more than emulating tab.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jul 11, 2020
@e-hein e-hein force-pushed the feature/emulate-arrow-keys branch 2 times, most recently from b92ddde to 3fd97c3 Compare July 11, 2020 19:14
@e-hein e-hein marked this pull request as draft July 12, 2020 07:33
@e-hein
Copy link
Contributor Author

e-hein commented Jul 12, 2020

did not expect tests to run in rtl direction :-/

@e-hein
Copy link
Contributor Author

e-hein commented Jul 19, 2020

ok, I really tried hard and I got no chance to reproduce the failed right-to-left tests on Edge 17. Seems not to happen in new Edge (based on chromium) anymore and I didn't find another browser with the same behavior. I can't find a possibility to downgrade Edge or get a machine which isn't updated yet.
I think I will have to wait until the next Edge-Update, so Edge 17 isn't in the list of supported browsers anymore :-(

@e-hein e-hein force-pushed the feature/emulate-arrow-keys branch 10 times, most recently from 6fb0cc9 to 51f72d9 Compare July 20, 2020 07:37
@mmalerba
Copy link
Contributor

We support the latest 2 versions of Edge (i.e. 84 & 83), as well as IE11. I think the Edge version running on our CI is just out of date.

@mmalerba
Copy link
Contributor

Actually if you pull from master, I believe the browsers have now been updated

@e-hein e-hein force-pushed the feature/emulate-arrow-keys branch from 51f72d9 to 8c5ffdf Compare July 25, 2020 10:15
e-hein added 3 commits July 26, 2020 09:58
- emulate cursor move on send arrow keys
- emulate selection on send shift + arrow key
- respect cursor position and selection on send characters

Prerequisite angular#19709 (will need select on tab into input with text)
Currently sending a key to a test input changes it's value even if default
event is prevented.

Now default event won't run if default is prevented
@e-hein e-hein force-pushed the feature/emulate-arrow-keys branch from 304a3a2 to 2c683ce Compare July 26, 2020 07:58
@e-hein e-hein marked this pull request as ready for review July 26, 2020 08:40
@e-hein e-hein requested a review from jelbourn as a code owner July 26, 2020 10:06
@mmalerba
Copy link
Contributor

@e-hein I talked to my TL about this and we're concerned about the amount of additional code we would have to maintain to support this, so I'm not sure we want to merge it into this repo. That said we definitely see how this is valuable and want to make sure that there's a good way for people to plug in behavior like this if they need it.

I think this could be accomplished by extending the UnitTestElement class and overriding the sendKeys and clear methods. You could then create a class that extends TestbedHarnessEnvironment and overrides the the createTestElement method to return instances of your custom test element. Does this sound like a reasonable solution to you? If you're interested I think it might be nice to publish this in a separate repo for others who might want more realistic key tab & arrow key handling in their tests.

@e-hein
Copy link
Contributor Author

e-hein commented Jul 31, 2020

@mmalerba thanks for your feedback. I also thought about that before I startet and I really understand your point. In fact I did publish to a separate repo before because of that point. I did not extend the UnitTestElement - you can have an independent sendKey-method (I called it emulateKey). spec sample / live sample. So this solution works for me, my projects and maybe other peoples projects, too ;-)

Why to prefer an integrated solution:

  • I would prefer a seamless solution where I do not have to think about which sendKey-Method to use
  • I really like your functions in dispatch-events.ts and element-focus.ts which are not part of the public api and very useful to emulate keys.
  • Test-Infrastructure and packaging. I will never get all that automated device and browser testing for my repo (only latest chrome and firefox on travis)
  • Of course it's easier to maintain and ensure compatibility if it's testet along with the framework.

Some options and alternatives to think about:

  • skip emulate arrows
    It's the largest part with the lowest impact (few people use arrows in specs) - but you will still have to take selection into account when changing values. So for me it felt some kind of incomplete without testing selections and therefor I need the arrow keys. emulate backspace or emulate tab are much smaller and much more important in tests.

  • alternative solution with less lines of code
    My first solution for arrow keys had only 59 lines of code. For left and right arrows it's working pretty find but I didn't like it because it's too complex and therefor difficult to maintain.

  • using emulate-key-in-browser as a dependency (no, I think I don't like this option, too - but I like complete lists)
    Then all 'emulate-*'-files and the code would be separated.

Finally 2 more points

  • Even if we do not change any code, I'd include a hint into the docs which describes the current behavior.
    I talkt about the current behavior and my issue to some colleagues and everyone had been really surprised that there are differences between environments.
  • If we do not change anything now and you change your mind in future, please drop me a note I would be happy to support this topic

@mmalerba
Copy link
Contributor

mmalerba commented Aug 3, 2020

@e-hein Thanks for the suggestions, I've added an agenda item to our next team meeting to discuss this. I do agree that there are some things we could do to make it a little easier for people who want to extend the keyboard emulation functionality

@mmalerba
Copy link
Contributor

After discussion this with the team we came to the following conclusions:

  • We don't want to set a precedent of trying to emulate everything the browser does, so this is not something we want to add to the library directly

  • We do think it would be good to expose some of the methods we have for working with events in tests. We've added an item to our project proposals list that will need to be prioritized. We don't want to just open up what we have now because it hasn't been fully thought through as a public API. The project proposal includes both the design and implementation work that would be needed.

  • We would certainly be open to clarifying the docs. I'm open to suggestions for how to improve the docs. We do make mention of it here: https://material.angular.io/cdk/test-harnesses/overview

    Please note that harnesses may not behave exactly the same in all environments. There will always be some difference between the real browser-generated event sequence when a user clicks or types in an element, versus the simulated event sequence generated in unit tests. Instead, the CDK makes a best effort to normalize the behavior and simulate the most important events in the sequence.

    Though clearly that may not be sufficient since your teammates missed it.

@e-hein
Copy link
Contributor Author

e-hein commented Aug 12, 2020

@mmalerba Thanks for discussing about the item and the detailed answer.
I like your result because it's a reliable statement to my question from June in #19709 - and even thought it's not the preferred result, I got now a basis to enforce development of tools to emulate browser behavior outside the angular components.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes PR author has agreed to Google's Contributor License Agreement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants