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.

Sign up for GitHub

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

Jump to bottom

Open up access to CharacterHit for subclasses of StyledTextArea #221

Merged
merged 4 commits into from
Dec 4, 2015
Merged

Open up access to CharacterHit for subclasses of StyledTextArea #221

merged 4 commits into from
Dec 4, 2015

Conversation

JordanMartinez
Copy link
Contributor

No description provided.

@TomasMikula
Copy link
Member

Hmm, I haven't noticed that ParagraphBox became public. I don't think it should be. Then only the hit(double x, double y) method can be made public.

@JordanMartinez
Copy link
Contributor Author

It's public even before the skin removal process. Should I make both the class and its constructor package private in this branch or via a new one?

@JordanMartinez
Copy link
Contributor Author

It's changed to public in this commit: 0ae84c2

@TomasMikula
Copy link
Member

Either way ;)

@JordanMartinez
Made ParagraphBox class package-private again: fixes small mistake or…
b3f984b 
…iginally made in commit "0ae84c25936bdb285434ecc96564f0b1f946221d"
@JordanMartinez
Copy link
Contributor Author

ParagraphBox's constructor is public long before this mistake. I'm guessing it's public so it could be accessed by the main richtext package outside the skin package?

However, now that it's inside the richtext package, it doesn't need to be public anymore.

Edit: actually... I guess that wouldn't make sense because if it wasn't public, then it couldn't be accessed by the richtext package anyways.... Either way, no errors arise when the constructor is no longer public.

@JordanMartinez
Made ParagraphBox Constructor package-private: no longer needs to be …
853a8e4 
…public now that skin has been removed and this class was moved to richtext package.
TomasMikula
TomasMikula reviewed Dec 4, 2015
View reviewed changes
richtextfx/src/main/java/org/fxmisc/richtext/StyledTextArea.java
@@ -677,7 +677,7 @@ CharacterHit hit(ParagraphBox.CaretOffsetX x, double y) {
}
}

CharacterHit hit(double x, double y) {
protected CharacterHit hit(double x, double y) {
Copy link
Member

Choose a reason for hiding this comment

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

Don't we want this one public and keep the other two package private?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess that would be better overall....

@JordanMartinez
Copy link
Contributor Author

I added javadoc that gives an example of what it can do, too.

TomasMikula added a commit that referenced this pull request Dec 4, 2015
@TomasMikula
Merge pull request #221 from JordanMartinez/characterHitAcess
9032e63 
Make StyledTextArea.hit(x, y) public.
@TomasMikula TomasMikula merged commit 9032e63 into FXMisc:master Dec 4, 2015
@TomasMikula
Copy link
Member

👍

@JordanMartinez JordanMartinez deleted the characterHitAcess branch December 5, 2015 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JordanMartinez @TomasMikula