First character in TextInput with `autoCapitalize` not capitalized

TheSavior commented 4 years ago

Description

The first character entered into a TextInput with autoCapitalize set isn't being capitalized. It does when typing manually.

I apologize that I haven't put in the effort to verify this against the latest release yet or given you a minimal repro. I hope that the attached video is more helpful than anything else.

[ ] I have tested this issue on the latest Detox release and it still reproduces

Reproduction

In React Native core I'm writing a test against our examples.

These are the tests I'm running:

it('Auto-capitalize should capitalize', async () => {
    await openExampleWithTitle('Auto-capitalize');
    const phrase = 'this is a long sentence. this is another.';

    await element(by.id('autocapitalize_none_input')).typeText(phrase);
    await element(by.id('autocapitalize_sentences_input')).typeText(phrase);
    await element(by.id('autocapitalize_words_input')).typeText(phrase);
    await element(by.id('autocapitalize_characters_input')).typeText(phrase);
  });

Provide the steps necessary to reproduce the issue. If you are seeing a regression, try to provide the last known version where the issue did not reproduce.

We are testing against these examples in RNTester (with testIDs added)

Expected behavior

I expect that for autoCapitalize of sentences, words, and characters would have the first character capitalized.

Video

(You may have to wait a moment for this gif to load and start playing. I promise it isn't a still frame) DetoxTextInputCapitalizationBug

Environment (please complete the following information):

Detox: 14.5.1 (we are currently in the process of updating)
React Native: master (e68946278273cc5ac9d79380a50a4bfcf384f25f)
Device: iPhone 8
iOS: 13.1

LeoNatan commented 4 years ago

Hello,

I am seeing a bug, but it's not what you expect. The typing mechanism is supposed to type the string provided in its exact form, and thus we attempt to disable any automatized text changes. I see now that we don't do it fully. From internal testing and here as well (if I recall correctly), this was deemed as the desired behavior, as Detox is not intended to test OS controls, but rather the app's response to the user's input.

https://github.com/wix/Detox/blob/52611248ad92c9cc2ced516af59833397e14f1bb/detox/ios/Detox/GREYActions%2BDetox.m#L72 I see now that the correct place to have this line is inside the loop, before typing each grapheme cluster.

We are currently working on a large rewrite of Detox. The main change is moving away from Earl Grey and using XCUITest. If I recall correctly, the above described is also the behavior of XCUIElement's typeText:.

TheSavior commented 4 years ago

Ah. Hmm. Are you saying that when Detox updates in my example all of the characters would be lower case?

That would be a bummer for our use case (arguably a very unique use case) as we wanted to be able to test that we wired up TextInput autoCapitalize correctly to the native views on each platform.

LeoNatan commented 4 years ago

That should be the behavior, but as you've shown in the gif, it's not working as expected beyond the first character due to my placement of that line. I might fix it (to be on every character), but probably not as I prefer we focus on the rewrite.

Regarding your scenario, hmmm... I wonder if somehow a unit test isn't a better tool to test this? I feel for you, but I just don't think this is the right way to test for this scenario. And since the future seems pretty set in this direction, I don't think we will add hacks in the current version (like some boolean perhaps to not disable the automatization), only to break them in a future version (when using XCUITest, we don't have access to the text field/view itself).

TheSavior commented 4 years ago

Yeah, I don’t think you should hack in a change for the current version that’s going to break in the future.

A unit test would typically make sense here, however it would need to be written for each platform.

We are trying to write a set of end to end tests that can be run against different platforms to verify they are consistent with the contract. That means iOS, Android, Windows, vr etc, and also against the new Fabric implementations for each of those.

It sounds like detox probably isn’t the right tool for that job.

I think it’s fine to close this issue as “won’t fix”.

wix / Detox