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

Refactor: Decouple style from segment #567

Closed
JordanMartinez opened this issue Aug 11, 2017 · 2 comments · Fixed by #590
Closed

Refactor: Decouple style from segment #567

JordanMartinez opened this issue Aug 11, 2017 · 2 comments · Fixed by #590

Comments

@JordanMartinez
Copy link
Contributor

The base area's type signature is:

public GenericStyledArea<ParagraphStyle, SegmentType, SegmentStyle>

When writing custom objects, we have to write the following code:

public CustomArea extends 
    GenericStyledArea<
        ParagraphStyle, 
        Either<
            TextObject<TextStyle>, 
            CustomObject<TextStyle>
        >, 
        TextStyle
    >

You'll notice that the styles are embedded into the custom objects. this doesn't make sense for two reasons:

  1. What if the custom object doesn't need a style (e.g. a hyperlink should always be styled as a hyperlink)
public CustomArea extends 
    GenericStyledArea<
        ParagraphStyle, 
        Either<
            TextObject<TextStyle>, 
            CustomObject<
                TextStyle   // This is useless for the custom object
            > 
        >, 
        TextStyle
    >
  1. What if one wanted to use a style objects for each object? In other words, Text could have TextStyle and CustomObject could have CustomObjectStyle? If so, we'd have to write this hairy code:
public CustomArea extends 
    GenericStyledArea<
        ParagraphStyle, 
        Either<
            TextObject<
                Either<
                    TextStyle, 
                    CustomObjectStyle
                >
            >, 
            CustomObject<
                Either<
                    TextStyle, 
                    CustomObjectStyle
                >
            >, 
        Either<
            TextStyle, 
            CustomObjectStyle
        >
    >
{}

I propose we separate the two, so that an segment type no longer knows its own style. Thus, the type signature would become:

public Case1Area extends 
    GenericStyledArea<
        ParagraphStyle, 
        Either<
            // stores the text, nothing more
            Text,
            // stores the displayed text and its actual link, nothing more
            Hyperlink 
        >, 
        // Text will render node using TextStyle. 
        // Hyperlink will ignore its associated TextStyle object 
        //    and set its node's style class to "hyperlink"
        TextStyle 
    >

public Case2Area extends 
    GenericStyledArea<
        ParagraphStyle, 
        Either<
            TextObject, 
            CustomObject
        >, 
        Either<
            TextStyle, 
            CustomStyle
        >
    >

// maybe a class like this that adds helper methods to work with the Eithers?
public GenericStyledEitherArea<PS, Seg1, Seg2, Style1, Style2> extends
    GenericStyledArea<PS, Either<Seg1, Seg2>, Either<Style1, Style2>>

Case2AreaAgain<ParagraphStyle, TextObject, CustomObject, TextStyle, CustomStyle>
    extends GenericStyledEitherArea< /* the types */ >
@JordanMartinez
Copy link
Contributor Author

One issue that can arise with this: setting the style of an Either.left object with an Either.right style.

@JordanMartinez
Copy link
Contributor Author

To me, the best way this should be resolved is by refactoring Paragraph, so that it stores the segments styles outside of them by having a separate StyleSpans<S> object. Then, the concat/subsequence/etc methods would call the corresponding methods on the List<SEG> segments object and StyleSpans<S> styles object each and pass their results into the Paragraph's constructor: Paragraph(paragraphStyle, segmentOps, segments, styles).

SegmentOps would need to refactored as well. TextExt would need to convert Paragraph<PS, SEG, S> into a List<StyledSegment<SEG, S>> to properly render the segments to the screen as a single style could be used throughout multiple segments, or a single segment, could have multiple styles.

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 a pull request may close this issue.

1 participant