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

Return correct key code for delete/backspace on android. #1362

Closed
wants to merge 1 commit into from

Conversation

Foaly
Copy link
Contributor

@Foaly Foaly commented Jan 29, 2018

This fixes #1309.

@eXpl0it3r eXpl0it3r added this to Backlog in Android Backlog via automation Jan 29, 2018
@eXpl0it3r eXpl0it3r added this to Discussion in SFML 2.5.0 via automation Jan 29, 2018
@eXpl0it3r eXpl0it3r added this to the 2.5 milestone Jan 29, 2018
@texus
Copy link
Contributor

texus commented Jan 30, 2018

This is a good example of why even trivial fixes need to be tested. The API has to be 13 or higher or it won't compile. The ANDROID_API_MIN currently still has a default value of 9, where AKEYCODE_FORWARD_DEL apparently didn't exist yet. I think the minimum version can be increased without any issues, even API 13 (android 3.2) is more than 7 years old already.
I couldn't test the delete key, but at least I can confirm that the backspace key works correctly now.

@Foaly
Copy link
Contributor Author

Foaly commented Jan 30, 2018

Funny somehow I had the gut feeling it would be a good idea to have this tested 😄
According to stats in the andoid dashboard the version 3.2 is not even listed anymore. Any significant user numbers don't start until 4.1.x. Unless anybody from the andoid devs (maybe @MarioLiebisch ) raises any concerns I will increase the API version.

Could you test the new version?

@Foaly Foaly force-pushed the bugfix/android_delete_key_fix branch from a4768e4 to a2bb2b4 Compare January 30, 2018 20:21
Copy link
Member

@MarioLiebisch MarioLiebisch left a comment

Choose a reason for hiding this comment

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

Latest tools will no longer allow you to build anything under API 14, so I'd say it's save to just bump the minimum version to 14 now.

@Foaly Foaly force-pushed the bugfix/android_delete_key_fix branch from a2bb2b4 to 0a8a2de Compare January 30, 2018 20:55
@Foaly
Copy link
Contributor Author

Foaly commented Jan 30, 2018

14 it is then!
I've also updated the references in the android example that I could find. I hope I didn't miss anything else.

@Foaly Foaly force-pushed the bugfix/android_delete_key_fix branch from 0a8a2de to 015c774 Compare January 30, 2018 20:58
@eXpl0it3r eXpl0it3r moved this from Discussion to Review & Testing in SFML 2.5.0 Jan 31, 2018
@texus
Copy link
Contributor

texus commented Feb 1, 2018

Works without problems now. I found a keyboard with a delete key so I can confirm that both backspace and delete works.

@Foaly
Copy link
Contributor Author

Foaly commented Feb 1, 2018

Thats awesome! Thank you very much for testing! I guess if @MarioLiebisch closes his review this can be merged :)

@MarioLiebisch
Copy link
Member

Yeah, changes are fine, but not sure we should wait for #1350 with this. You'll basically need both changes to properly compile without this bug (and others).

@Foaly
Copy link
Contributor Author

Foaly commented Feb 8, 2018

I don't really see how #1350 is dependent on this, so I think this is ready for merging now! :)

@eXpl0it3r eXpl0it3r moved this from Review & Testing to Ready in SFML 2.5.0 Feb 11, 2018
@eXpl0it3r
Copy link
Member

Small conflict to resolve since the other Android bit has been merged. 🙂

Also increased minimum Android API version to 14.
@Foaly Foaly force-pushed the bugfix/android_delete_key_fix branch from 015c774 to b1294d2 Compare February 11, 2018 21:39
@Foaly
Copy link
Contributor Author

Foaly commented Feb 11, 2018

Ah I see it now :D Updated!

@eXpl0it3r
Copy link
Member

Merged in 1862946

@eXpl0it3r eXpl0it3r closed this Feb 15, 2018
SFML 2.5.0 automation moved this from Ready to Merged / Superseded Feb 15, 2018
Android Backlog automation moved this from Backlog to Done Feb 15, 2018
@Foaly Foaly deleted the bugfix/android_delete_key_fix branch February 15, 2018 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
SFML 2.5.0
  
Merged / Superseded
Development

Successfully merging this pull request may close these issues.

Android sends Delete event instead of Backspace
4 participants