-
Notifications
You must be signed in to change notification settings - Fork 236
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
Add Hyperlink demo - minimum needed for custom object integration #550
Conversation
|
||
import java.util.function.Consumer; | ||
|
||
public class TextHyperlinkArea extends GenericStyledArea<ParStyle, Either<StyledText<TextStyle>, Hyperlink<TextStyle>>, TextStyle> { |
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.
This demo seems like a lot of code just to demo hyperlinks. And it isn't clear to me at a quick glance if something like ParStyle is part of the minimal code necessary to create a hyperlink or if it's part of the bells and whistles of the demo.
Not to disparage your work, it's great that you put this together.
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.
Not to disparage your work, it's great that you put this together.
None taken. I appreciate your feedback!
You are correct. ParStyle
isn't needed at all.
If Java supported algebraic data types, writing the SegmentOps
and real/empty hyperlink data types would be a lot easier to code... but it doesn't :-/
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.
I also reworked StyledText
's equals
method to return true when both text objects are empty since createText
will sometimes create an empty StyledText object but not use the one we want it to use (emptySeg).
Also, I'm not sure if our current pattern/approach is correct in one aspect: the empty segment's style. Technically, the segment is only empty in text, but not in style. The style is used when inserting text without specifying the style. The code will look at the style at the position of insertion and use that style for the text being inserted.
I think we need to update the code to use this approach:
public Style getStyleForInsertion(int position) {
if (useInitialTextstyle) {
return initialTextStyle;
} else {
return getStyleAtPosition(position);
}
}
public Style getStyleAtPosition(int position) {
Optional<Style> s = getStyleAtPosition0(position);
if (s.isPresent()) {
return s.get();
} else {
return getStyleAtPosition(position - 1);
}
}
public Optional<Style> getStyleAtPosition0(int position) {
Style style = content.getStyleAtPosition(position);
return Optional.ofNullable(style);
}
|
||
import org.fxmisc.richtext.model.SegmentOpsBase; | ||
|
||
public class HyperlinkOps<S> extends SegmentOpsBase<Hyperlink<S>, S> { |
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.
Where does SegmentOpsBase come from? I don't see it in here, and my compiler isn't finding it.
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.
I added that in #553 since I realized that it's easier just to tell developers to extend a base class that already properly overrides everything than to tell them to copy/paste the same pattern in the wiki page.
No release currently has that, but you can get it if you follow the Jitpack instructions on the ReadMe.
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.
Just FYI. The updated version doesn't use SegmentsOpsBase
It seems that even the empty segment object needs to be able to update its style because of what I mention in my comment to your previous one. This is why I'm proposing #567.
This version follows Also, when concatting two hyperlinks together that share the same ancestor, it will form one hyperlink. |
Shows custom object support in a less-cluttered way than the RichTextDemo.