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

8245499: Text input controls should show handles on iOS #231

Closed
wants to merge 4 commits into from

Conversation

jperedadnr
Copy link
Collaborator

@jperedadnr jperedadnr commented May 21, 2020

After JDK-8242167, a JavaFX control is used for text input on iOS instead of the native control, on a touch enabled device. However, selection handles are not enabled and currently text selection is not possible at all.

This PR enables handles on iOS as in the rest of the platforms with touch support.

It also tries to accommodate its style to the native style for handles, where selection handles have a circle shape and are a little bit bigger, and there is no caret handle.

I've tested it on iPhone, running a small HelloFX sample, with a TextField control.


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

  • JDK-8245499: Text input controls should show handles on iOS

Reviewers

  • Ajit Ghaisas (aghaisas - Reviewer)
  • Johan Vos (jvos - Reviewer)

Download

$ git fetch https://git.openjdk.java.net/jfx pull/231/head:pull/231
$ git checkout pull/231

@bridgekeeper
Copy link

bridgekeeper bot commented May 21, 2020

👋 Welcome back jpereda! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request.

@openjdk openjdk bot added the rfr Ready for review label May 21, 2020
@mlbridge
Copy link

mlbridge bot commented May 21, 2020

Webrevs

@kevinrushforth
Copy link
Member

/reviewers 2

@openjdk
Copy link

openjdk bot commented May 21, 2020

@kevinrushforth
The number of required reviews for this PR is now set to 2 (with at least 1 of role reviewers).

@kevinrushforth
Copy link
Member

Since this touches shared code, I'd like @aghaisas to look at this.

@aghaisas
Copy link
Collaborator

Code changes look OK.
I think, we need a note in PR description about what testing you have done with this fix.

@jperedadnr
Copy link
Collaborator Author

I've edited the PR description with details of the tests I've done.

@openjdk
Copy link

openjdk bot commented May 27, 2020

@jperedadnr This change now passes all automated pre-integration checks. When the change also fulfills all project specific requirements, type /integrate in a new comment to proceed. After integration, the commit message will be:

8245499: Text input controls should show handles on iOS

Reviewed-by: aghaisas, jvos
  • If you would like to add a summary, use the /summary command.
  • To credit additional contributors, use the /contributor command.
  • To add additional solved issues, use the /issue command.

Since the source branch of this PR was last updated there have been 7 commits pushed to the master branch:

  • 8914bd2: 8234540: javafx.web LeakTest.testGarbageCollectability fails intermittently
  • 16f446a: 8234876: Unit test classes should not extend Application
  • 2d98fe6: 8245601: TESTBUG] Disable TabPaneDragPolicyTest on Mac until JDK-8213136 is fixed and fix ISE
  • f3190db: 8244531: Tests: add support to identify recurring issues with controls et al
  • 1971c70: 8245457: TestBug] Enable and fix ignored tests in ButtonBaseTest & ButtonTest
  • 6e0b45a: 8245183: Two fxml unit tests log warnings about deprecated escape sequences
  • a13a642: 8244579: Windows "User Objects" leakage with WebView

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid automatic rebasing, please merge master into your branch, and then specify the current head hash when integrating, like this: /integrate 8914bd2a7fe442239922ceecb6678d1e17549f1e.

As you do not have Committer status in this project, an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@aghaisas, @johanvos) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

@openjdk openjdk bot added the ready Ready to be integrated label May 27, 2020
@jperedadnr
Copy link
Collaborator Author

/integrate

@openjdk
Copy link

openjdk bot commented May 27, 2020

@jperedadnr
Your change (at version fc681a1) is now ready to be sponsored by a Committer.

@openjdk openjdk bot added the sponsor Ready to sponsor label May 27, 2020
@aghaisas
Copy link
Collaborator

/sponsor

@openjdk openjdk bot closed this May 27, 2020
@openjdk openjdk bot added integrated Pull request has been integrated and removed sponsor Ready to sponsor ready Ready to be integrated rfr Ready for review labels May 27, 2020
@openjdk
Copy link

openjdk bot commented May 27, 2020

@aghaisas @jperedadnr The following commits have been pushed to master since your change was applied:

  • 8914bd2: 8234540: javafx.web LeakTest.testGarbageCollectability fails intermittently
  • 16f446a: 8234876: Unit test classes should not extend Application
  • 2d98fe6: 8245601: TESTBUG] Disable TabPaneDragPolicyTest on Mac until JDK-8213136 is fixed and fix ISE
  • f3190db: 8244531: Tests: add support to identify recurring issues with controls et al
  • 1971c70: 8245457: TestBug] Enable and fix ignored tests in ButtonBaseTest & ButtonTest
  • 6e0b45a: 8245183: Two fxml unit tests log warnings about deprecated escape sequences
  • a13a642: 8244579: Windows "User Objects" leakage with WebView

Your commit was automatically rebased without conflicts.

Pushed as commit 3ceee69.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrated Pull request has been integrated
4 participants