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

[iOS/TextInput] Impossibility to use inputAccessoryView #16071

Closed
douglasjunior opened this issue Sep 23, 2017 · 11 comments
Closed

[iOS/TextInput] Impossibility to use inputAccessoryView #16071

douglasjunior opened this issue Sep 23, 2017 · 11 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@douglasjunior
Copy link

douglasjunior commented Sep 23, 2017

Is this a bug report?

It is a incorrect behavior introduced in version 0.47.0, commit 2b1795c

This commit brought a problem to those who use a custom ToolBar in TextInput, like IQKeyboardManager. And there is no option to disable this behavior.

Environment

Environment:
OS: macOS Sierra 10.12.6
Node: 6.11.0
Yarn: 1.0.2
npm: 5.4.2
Watchman: 4.9.0
Xcode: Xcode 9.0 Build version 9A235
Android Studio: EAP AI-171.4316950 AI-171.4316950

Packages: (wanted => installed)
react: 16.0.0-alpha.12 => 16.0.0-alpha.12
react-native: 0.48.2 => 0.48.2

Steps to Reproduce

  1. $ clone https://github.com/douglasjunior/react-native-inputAccessoryView.git
  2. $ cd react-native-inputAccessoryView
  3. $ yarn install
  4. $ react-native run-ios

Expected Behavior

It is expected that when typing, the Toolbar will not disappear. As in react-native 0.46.4.

correct

Actual Behavior

When typing, the Toolbar disappears, because the inputAccessoryView is set to null.

incorrect

douglasjunior referenced this issue Oct 2, 2017
…n iOS

Summary:
Standard only-numeric (number pad) keyboard on iOS does not have any "Done" or "Enter" button, and this is often very badly hurt user experience.
Usually it can be solved by implementing custom `inputAccessoryView`, but RN does not have built-in support for customizing it.
So, this commit introduced limited support only for "Done" button (returnKeyType="done") and it should suite very well for the vast majority of use cases.
This is highly requested feature, see more details here:
#1190

Reviewed By: mmmulani

Differential Revision: D5268020

fbshipit-source-id: 90bd5bffac6aaa1fb7c5c2ac539b35b04d45918f
@shergin
Copy link
Contributor

shergin commented Oct 2, 2017

I thought about this issue for awhile after it was reported as a commit comment. And I think that we should not fix this in RN.
We should not because this is a problem inside IQKeyboardManager, not inside RN.
IQKeyboardManager has (controversial) "magical" design that assumes that it is okay to interfere with actual product code (which actually is unaware of this). So, if the authors of this framework went this way (which is perfectly fine), they need to add even more magic to workaround this problem.
From other side, even if can "fix" this issue on RN side, we should not overcomplicate internal logic just to be compatible with one specific library.

@shergin shergin closed this as completed Oct 2, 2017
@douglasjunior
Copy link
Author

Sorry, I dont think that you understood the problem, and I dont think that issue should be closed.

  1. I think that the feature of managing the inputAccessoryView should not be responsability of the RN TextInput. And if it is, there should be an option for disable this. It's an evasive feature, it broke the default behavior.

  2. The problem here is not with IQKeyboardManager, I just used it as an example. Today you can not use an inputAccessoryView anyway. It's impossible. So I think this is a problem in the RN.

  3. This functionality broke the framework to add irrelevant value to it.

There is no magic, what the library does is simple, and the library is not the focus of the problem here. In conclusion, I just want to be able to add my own inputAcessoryView. Today I can not.

@shergin
Copy link
Contributor

shergin commented Oct 3, 2017

Let me put it this way: It is RN's UITextView/UITextField instance, RNText lib owns this and controls it. If some another library access it directly and the result is undesirable, it should be fixed in that library.

@douglasjunior
Copy link
Author

douglasjunior commented Oct 3, 2017

Forget the library, the question here is that I cant use my own inputAccessoryView.

Before using any library I, and several others developers, used an own inputAccessoryView to create navigation buttons and offer a better UX. RN simply put an end to it, in an undemocratic way.

I worked on this problem for a few days and could not find a solution. Because the RN overrides the inputAccessoryView and there is nothing that can be done externally.

I just want to make sure if you really understand the problem and are if you making the right decision. Everything would be resolved with an option to disable this new "feature".

My suggestion is to create an option that will change this:

- (void)didSetProps:(NSArray<NSString *> *)changedProps
 {
   [self invalidateInputAccessoryView];
 }

To something like this:

- (void)didSetProps:(NSArray<NSString *> *)changedProps
 {
   if (automaticToolbarEnabled) {
        [self invalidateInputAccessoryView];
   }
 }

@shergin
Copy link
Contributor

shergin commented Oct 5, 2017

Okay, I will fix that, I think I know how to do it.
I am doing it because:

  • Seems it will not increase complexity of current implementation;
  • I cannot guarantee that it will work forever because... see the previous point;
  • RN does not support customizing inputAccessoryView;
  • I love you, guys.

@shergin shergin reopened this Oct 5, 2017
@douglasjunior
Copy link
Author

douglasjunior commented Oct 5, 2017

I really appreciate your help 🎉 . We love you too ❤️ .

I have tried to solve this in several ways, but I could not. My temporary bad solution was to create an extension to override the invalidateInputAccessoryView.

Please, if you have any documentation that can help me to understand the flow of RN internal components, I can try to help you better in the next time.

facebook-github-bot pushed a commit that referenced this issue Oct 9, 2017
Summary:
Now RCTTextInput relies on ivar to detect should it reconstruct inputAccessoryView or not.
Previously we checked `inputAccessoryView` directly which breaks some 3rd party libs.

See #16071 for more details.
cc douglasjunior

Reviewed By: javache

Differential Revision: D5994798

fbshipit-source-id: c086efdd24f5528393a4c734ff7c984e0ed740d1
@shergin
Copy link
Contributor

shergin commented Oct 10, 2017

@douglasjunior bf36983 should fix your problem.

@shergin shergin closed this as completed Oct 10, 2017
@douglasjunior
Copy link
Author

Amazing, thank you @shergin. 🎉

@douglasjunior
Copy link
Author

douglasjunior commented Oct 10, 2017

I tested it with the master branch and it really worked. However when using numeric input the RN inputAcessoryView will override the custom inputAcessoryView yet. But I think this can be solved in other ways.

@rplankenhorn
Copy link

@douglasjunior Can you elaborate on how you accessed inputAccessoryView? My project is currently dependent on react-native-textinput-utils and I would like to remove this dependency since the repo is extremely out of date. Did you go with the extension approach you mentioned above?

@douglasjunior
Copy link
Author

@facebook facebook locked as resolved and limited conversation to collaborators Oct 10, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Oct 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

4 participants