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

Bug fixed 216 #240

Closed
wants to merge 6 commits into from
Closed

Conversation

JordanMartinez
Copy link
Contributor

I think the final commit (4d86628) could be done better. However, I'm not sure how to get around a plumbing issue.
If a new EmptyParagraph is created anywhere outside of EditableStyledDocument, it doesn't have the correct style object for its emptyTextStyle (e.g. StyledDocumentBase, NormalParagraph, ReadOnlyStyledDocument). Additionally, its paragraphStyle isn't always correct either when it is created in some method in StyledDocument and ReadOnlyStyledDocument.

I think the only good solution is to remove EmptyParagraph's emptyTextStyle field and use the StyledDocument classes to check the Paragraph type before returning a style object, such as:

class StyledDocumentBase {

    StyledDocumentBase(L paragraphs, S initialStyle, PS initialParagraphStyle) {
        // normal creation code
        this.initialStyle = initialStyle;
        this.initialParagraphStyle = initialParagraphStyle;
    }

    Paragraph someOtherMethodThatCreatesANewEmptyParagraph() {
        return new EmptyParagraph(initialParagraphStyle);
    }

    S getStyleOfChar/getStyleAtPosition(int position) {
        Paragraph<S, PS> par = paragraphs.get(position);
        return par instanceof EmptyParagraph
            ? initialStyle
            : par.getStyleOfChar/getStyleAtPosition(position);
    }
}

Doing the above would require another change. ClipboardActions has a default method that creates a ReadOnlyStyledDocument, but it doesn't have any reference within itself for getting the two styles for its new constructor. So, it'd also need to have two getters for the styles. Since it's name is ClipboardActions, I wasn't sure if something should further extend that. Possibly an interface called StyledClipboardActions ?

…sier to understand when interface `Paragraph<S, PS>` is implemented .
…graph to Paragraph.

EditableStyledDocument details
- Constructor now initializes itself with an EmptyParagraph
- Replace method checks for NormalParagraphs that should be EmptyParagraphs because their length == 0
- getStyleForInsertionAt(Position) will return `initialStyle` if that boolean property is true or if the Paragraph returned is EmptyParagraph

ReadOnlyStyledDocument details
- The following were adjusted to account for saving/loading an EmptyParagraph or NormalParagraph:
 - fromString() method
 - Codecs
…om NormalParagraph to Paragraph

ParagraphText details
- When ParagraphText is displaying an EmptyParagraph, it will seem like the paragraph and the caret aren't visible. To fix this, an empty TextExt was added. There is probably a better way to make the line and caret appear. Additionally, the caret's height might not appear to be the same elsewhere in the area if the initialStyle used in EditableStyledDocument doesn't specify what the fontsize should be of the text.
- There's probably now a way to show that a newline char is highlighted when user selects something by `new TextExt(" ")` rather than `new TextExt("")`. For example, when a user selects multiple lines and one of them is an EmptyParagraph, the TextExt could be used to show that that line is selected. As of now, there is no indication of this to the user.
…o that `StyledDocument`s that are used to initialize a new `RichTextChange` don't have any `NormalParagraph` instances in their respective paragraphs when those instances length is 0. Instead, remaps the paragraphs to `EmptyParagraph` objects using `EditableStyledDocument`'s `initialStyle` and `initialParagraphStyle`.
@JordanMartinez
Copy link
Contributor Author

Once fully implemented, I think some tests should be written for the Paragraph methods that return a Paragraph to insure that EmptyParagraph is returned instead of a NormalParagraph whose length is 0 because I'm pretty sure that part isn't error-free yet in this branch.

@JordanMartinez
Copy link
Contributor Author

Closing as EmptyParagraph/NonEmptyParagraph approach to solving bug is "orthogonal" (#216).

@JordanMartinez JordanMartinez deleted the bugFixed-216 branch January 23, 2016 00:05
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.

1 participant