Skip to content

support superscript at the end of the text - #95

Closed
VakhoQ wants to merge 4 commits into
vandeseer:developfrom
VakhoQ:support-superScripts
Closed

support superscript at the end of the text#95
VakhoQ wants to merge 4 commits into
vandeseer:developfrom
VakhoQ:support-superScripts

Conversation

@VakhoQ

@VakhoQ VakhoQ commented Jul 14, 2020

Copy link
Copy Markdown
Contributor

I added functionality for SuperScritp.

Usage:

 
  .text("Hello World").borderWidth(4).backgroundColor(Color.WHITE)
  .superScript(TextCell.builder().text("2").fontSize(6).textRise(5).build())
 

By default textRise is 0.

Note, right now only TextCell supports this functionality. In the future , I think we need abstract methods in the AbstractTextCell to avoid casting and checking instances:

    public abstract boolean hasSuperScript();
    public abstract AbstractTextCell superText();

Under the hood we have the following:

contentStream.setTextRise(styledText.getTextRise());

Additionally, I add logic to move text to the next line if super-text does not fit to the current column.

@VakhoQ VakhoQ changed the title support super scripts support superscript Jul 14, 2020
@VakhoQ VakhoQ changed the title support superscript support superscript at the end of the text Jul 15, 2020
@vandeseer

Copy link
Copy Markdown
Owner

Hi @VakhoQ, thanks for your pull request.

I will look at it a bit closer as soon as I find time for it. That could last a bit though, I am pretty busy at the moment.

Best,
Stefan

@VakhoQ

VakhoQ commented Jan 26, 2021

Copy link
Copy Markdown
Contributor Author

@vandeseer
Hi. Let me ask you if you had chance to look into the pull request?

@vandeseer

Copy link
Copy Markdown
Owner

Hey @VakhoQ,

I am really sorry not having had a look earlier!

There are some general remarks I have now after a first look: As you already said there is a lot of casting going on and it feels a bit hacky the way it is implemented right now. Reason being that is implemented within TextCell. What I mean by "hacky" is:

                if(cell instanceof TextCell){
                    TextCell textCell = (TextCell) cell;
                    if(textCell.hasSuperScript()){
                        textCell= textCell.getSuperScript();
                        textCell.getSettings().fillingMergeBy(row.getSettings());
                    }
                }

This is not very elegant. So I would suggest the following approach: Why not using a cell type of its own? E.g. SuperScriptCell or maybe (to be a bit more generic and allow for further similar changes): ExtendedTextCell or something similar.
Then there is no need for such casts on the one hand side and and secondly it's implementation is more independent. There would be some kind of "basic text cell" and another one that is all of that plus some extras.

@vandeseer

Copy link
Copy Markdown
Owner

Closed due to #125

@vandeseer vandeseer closed this May 22, 2021
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