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

Adding text input command #137

Merged
merged 5 commits into from
Feb 21, 2016
Merged

Conversation

tecknut
Copy link
Contributor

@tecknut tecknut commented Feb 15, 2016

Additions

  • Adds an input command which sets text on text views or text fields which were first responder when execution paused.
  • Adds setxt command which sets text on a view by accessibility ID.

Example

(lldb) input 'Some text to set.'
(lldb) setxt some.accessibility.id 'Some text to set.'

self.findFirstResponder(subview, replacementText)
else:
if isFirstResponder(view):
setTextInView(view, replacementText)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My python is clearly lacking, how does this else: section work? Does it only run if the for loop has nothing to loop over (in this case view count is zero)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that's exactly it. My python isn't that great either, only found out about this while writing this code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't handle any first responder that has subviews. Is there a reason for that? I'm not aware of anything that prevents a view with subviews from being first responder.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This command is intended to be used after selecting a text field/text view, making it first responder, then setting the text on it.

With that in mind, if anything else is first responder at the time this command is run it will either do nothing, or set the text property on it.

I took the happy path for this, but I could put checks in the setTextInView function which only set text on classes which respond to setText:.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just looking back over the code, this branch of code is only executed when a view has no subviews.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this branch of code is only executed when a view has no subviews.

Right, and I'm wondering about cases where some text input view has internal subviews. There's no guarantee that that a text input view will always have no subviews. I think the isFirstResponder check should go above the loop, and if it fails, then loop over subviews. This would make it breadth first, but I don't see a problem with that, do you?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, no issue. Pushed it up.

@kastiglione
Copy link
Contributor

Thanks @tecknut for the pull request!

@kastiglione
Copy link
Contributor

I'm going to give this a day or two to see if @gkassabli has an feedback or thoughts about the accessibility part.

@gkassabli
Copy link
Contributor

Looks good to me

kastiglione added a commit that referenced this pull request Feb 21, 2016
@kastiglione kastiglione merged commit 9e26eb8 into facebook:master Feb 21, 2016
@kastiglione
Copy link
Contributor

Thanks!

@tecknut
Copy link
Contributor Author

tecknut commented Feb 21, 2016

Thanks guys !

@tecknut tecknut deleted the adding-input-command branch February 21, 2016 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants