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

Add Hyperlink demo - minimum needed for custom object integration #550

Merged
merged 1 commit into from
Aug 11, 2017
Merged

Add Hyperlink demo - minimum needed for custom object integration #550

merged 1 commit into from
Aug 11, 2017

Conversation

JordanMartinez
Copy link
Contributor

@JordanMartinez JordanMartinez commented Jul 31, 2017

Shows custom object support in a less-cluttered way than the RichTextDemo.


import java.util.function.Consumer;

public class TextHyperlinkArea extends GenericStyledArea<ParStyle, Either<StyledText<TextStyle>, Hyperlink<TextStyle>>, TextStyle> {
Copy link

@stevedefazio stevedefazio Aug 9, 2017

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.

Copy link
Contributor Author

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 :-/

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 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> {

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.

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 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.

Copy link
Contributor Author

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.

@JordanMartinez
Copy link
Contributor Author

This version follows StyledText in that it doesn't have a real and empty version of the same object.

Also, when concatting two hyperlinks together that share the same ancestor, it will form one hyperlink.

@JordanMartinez JordanMartinez changed the title Hyperlink demo Add Hyperlink demo - minimum needed for custom object integration Aug 11, 2017
@JordanMartinez JordanMartinez merged commit 99770e5 into FXMisc:master Aug 11, 2017
@JordanMartinez JordanMartinez deleted the hyperlinkDemo branch August 11, 2017 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants