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
Fixes https://github.com/react-native-webview/react-native-webview/is… #3307
base: master
Are you sure you want to change the base?
Conversation
Hello I haven't made extensive testing on new arch, I'll hopefully review this soon! |
} | ||
|
||
@Override | ||
public void requestFocus(RNCWebViewWrapper view) { | ||
view.requestFocus(); | ||
// NOTE: It is handled via receiveCommand(). | ||
// view.requestFocus(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of leaving empty overrides?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@o-alexandrov I don't have previous experience with native UI components, and I haven't looked much into how exactly that codegenNativeCommands()
and .receiveCommands()
stuff works. Just looking at the existing code and behavior it looked to me that if these .requestFocus()
, etc. methods get called directly, perhaps they are the best way to implement related functionality (and probably they are abstract in a parent class, thus have to be overridden anyway — but I haven't really checked it). Then, there is the backward compatibility to the old arch, I thought if I touch .receiveCommands()
, perhaps it is needed for the old arch, and it will need a bunch more efforts to ensure it works there.
My immediate interest here was just to hotfix the bug causing me troubles, thus I figured out this is the minimal intrusion way to do it — comment out the calls that cause duplicated command executions, but otherwise leave these method in the codebase so that somebody later looks second time into it.
This seems like an important issue to get addressed. Is there any progress on this front? Or anything in particular blocking progress? |
Hello 👋, this PR has been opened for more than 2 months with no activity on it. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically |
bot go away! |
Fixes #3305.