LOOP-1916: Move cursor to the end on begin editing (take 2)

[rickpasetto]

Pete gave good PR feedback that the caller should be responsible for deciding whether to queue on .main, not the callee.
In this case, too, since the callers were already on main, but still need to queue due to timing issues, I added comments explaining why.

[ps2]

LGTM, though I'm a little worried the fix is being applied too wide.

[ps2]

This cell is used a lot of places in Loop, I think, and I haven't done a review to see if we really want this behavior on all of them. Some places, like dexcom code entry, the view is centered and the font is large, and the standard iOS behavior of the cursor landing where you tap is useful and easy to do, and probably expected. The places where we've had problems and really want the cursor to go to the end is in views where 1) making a too big value accidentally by prepending instead of appending is risky, 2) where the input is relatively small, and/or 3) the value is on the right hand side of the screen, making placement of the cursor on the right more difficult.

[ps2]

I'm wondering if we should add this behavior under a flag, or do a review of what interfaces this affects.

[rickpasetto]

IMHO consistency is better here. I humbly disagree that this behavior should vary between different text field entry UI. That would only make matters more confusing for the user.

(P.S. The Dexcom code entry screen works fine, because it always starts empty.)

[ps2]

Cursor placement is an iOS idiom; we're breaking convention and already ensuring inconsistency by forcing the cursor to the right. The user is spending their time in a system where our input fields, or at least those we choose to apply this too, will be the odd duck out, and this may be a confusing situation. I think the safety concerns we have seen for certain interfaces warrant this non-standard behavior, but I'm just wondering if we want to scope the damage.

[ps2]

And as for Dexcom code inputs; it's the same as other fields; it's only when editing a value that the issue arises. You may put in a code, realize it's wrong, want to edit it, but you can't do what you expect, and you end up having to delete and re-enter several valid values to fix the one you want to change.

[rickpasetto]

I disagree that we're breaking iOS convention. I've used many apps that vary this behavior, from cursor-on-the-right, to cursor-on-the-left, to "select the whole text" field to edit it. While I haven't done a full assay on apps out there, I can confidently say this is true. So making statements like this is "non-standard" behavior that will cause "damage" is, I believe, overstating things a bit.

This PR is not changing anything. It is only addressing one of the architectural concerns you gave in the previous PR (#243). I recommend we merge this and continue to monitor the situation.

Re: the Dexcom code entry: I don't understand the steps you outline to reproduce. There's no "editing the value" in this case. It always starts empty.

[ps2]

It's definitely a convention. I just tried several apple apps (calendar, apple health, contacts) and different fields, and they all allowed placement of cursor on the right or left, and hover to place somewhere in the middle. I also just tried the dexcom cgm manager code entry, and was able to enter a code, then place the cursor in the middle, delete one digit, and replace it.

I'm not saying that this PR needs to be changed (I did approve it), but I do want to call attention to the fact that the related changes that were landed in the previous PR are addressing the very real problem in some UIs with a broad brush that affects other UIs that we haven't had issues with, and are now causing them to behave in non-standard ways, and yes, if this causes user confusion where we didn't need to, I do think it is damage.

[rickpasetto]

You can still do all of those things with the changes here (and in the prior PR). All I changed is the initial cursor placement. Maybe I don't understand your concern fully.

[rickpasetto]

I'm happy to revisit this at dev sync if you'd like. Feel free to bring it up.

…

On Mon, Sep 21, 2020 at 2:47 PM Pete Schwamb ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In LoopKitUI/Views/TextFieldTableViewCell.swift <#245 (comment)>: > guard let self = self else { return } + self.textField.moveCursorToEnd() It's definitely a convention. I just tried several apple apps (calendar, apple health, contacts) and different fields, and they all allowed placement of cursor on the right or left, and hover to place somewhere in the middle. I also just tried the dexcom cgm manager code entry, and was able to enter a code, then place the cursor in the middle, delete one digit, and replace it. I'm not saying that this PR needs to be changed (I did approve it), but I do want to call attention to the fact that the related changes that were landed in the previous PR are addressing the very real problem in some UIs with a broad brush that affects other UIs that we haven't had issues with, and are now causing them to behave in non-standard ways, and yes, if this causes user confusion where we didn't need to, I do think it *is* damage. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#245 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACLOAPYXXQHWI4FYC7YK53SG7CXJANCNFSM4RSNCB2Q> .