Hi,

I've looked over this and I don't see any issues - meaning gotchas.
It just provides a way to over-ride the hard-coded 8,
whether using a Text node directly or a TextFlow.

Two observations of what one might call limitations
1) This is a rendering time property, which is controlled by the program.
A document containing tabs saved and re-rendered somewhere else
won't be helped.

2) This just provides API on the scene graph node types, not any of the UI controls
which use Text nodes. Something like a CSS property may be the way to go if
you wanted that.
Text has a nested class StyleableProperties for CSS properties with which it
plays nice : font, underline, strikethrough, text-alignment

So creating an fx-tabWidth (or similar name) CSS property could be propagated
through to there in a similar way.

-phil.


On 9/30/19 9:48 AM, Scott Palmer wrote:
Okay, current work relocated to a clone of the new official Git repo.
My initial implementation is here:
  https://github.com/swpalmer/jfx/commit/cc6193451bf8a693093f3ded5dcbe47af2fcbe8f 
<https://github.com/swpalmer/jfx/commit/cc6193451bf8a693093f3ded5dcbe47af2fcbe8f>

I would submit it as a pull request but that seems premature since there hasn’t 
been any real discussion of challenges I’ve overlooked.  All I have are the 
famous last words, “It works for me.”

I saw in the archives a concern about tab width vs tab stops. For some reason I 
didn’t get the email.  Anyway, I don’t think arbitrary tab stop positions, at 
different intervals that is, are what was asked for.  That certainly would 
require a significantly different implementation.

Would love to keep some momentum going on this before I become busy with other 
things and won’t have time for it.

Scott

On Sep 23, 2019, at 3:57 PM, Scott Palmer <swpal...@gmail.com> wrote:

My current work is here 
https://github.com/javafxports/openjdk-jfx/compare/develop...swpalmer:jdk-8130738?expand=1
 
<https://github.com/javafxports/openjdk-jfx/compare/develop...swpalmer:jdk-8130738?expand=1>

While writing a unit test I realized that the StubToolkit isn’t really running 
the Prism layout code, so I’m not sure how that gets tested.  I made it work 
with StubTextLayout for now.

Regards,

Scott


On Sep 20, 2019, at 12:57 PM, Scott Palmer <swpal...@gmail.com 
<mailto:swpal...@gmail.com>> wrote:

Thanks Kevin.

My current implementation appears to be working for TextFlow and Text, with the 
TextFlow overriding the tabWidth of the child Text nodes.  This seems to work 
out naturally from the way TextFlow overrides the TextLayout instance used by 
the Text node.

If there are tricky corner-cases that I’m missing, I guess figuring out all the 
cases it will need to handle is part of this discussion.  It didn’t seem to be 
that challenging so far, so perhaps I am missing something (hopefully not).  I 
wrote a small test app to visually see that things were working as I expected.  
I have not yet written the unit tests.

Cheers,

Scott

On Sep 20, 2019, at 10:58 AM, Kevin Rushforth <kevin.rushfo...@oracle.com 
<mailto:kevin.rushfo...@oracle.com>> wrote:

Hi Scott,

I'm sure Phil will have more comments on this. While the API seems simple 
enough on the surface, I suspect that this will be a challenging feature to 
implement correctly for all of the cases it will need to handle. It would need 
careful review and testing as well. My only comment on the API itself is that 
if we do accept this feature, it should probably go on both Text and TextFlow, 
and be one of the attributes of Text that is ignored / overridden when a Text 
node is in a TextFlow.

-- Kevin


On 9/18/2019 6:14 PM, Scott Palmer wrote:
I would like to implement this feature, being able to adjust the tab size in a TextFlow or 
Text node (JDK-8130738 <https://bugs.openjdk.java.net/browse/JDK-8130738 
<https://bugs.openjdk.java.net/browse/JDK-8130738>>).  It involves new public API, 
so I want to start a discussion about it here.  (My motivation is that RichTextFX suggests 
an entirely unacceptable workaround of substituting actual spaces when the tab character is 
typed and cites the lack of this API.)

I’ve already jumped the gun and taken a crack at an implementation.  It is 
currently incomplete as I was just poking around to see if it was going to be 
easy enough to not take up too much of my time.  It boils down to:
TextFlow and Text get a new property for tab width, an integer representing the 
number of spaces taken by a tab. (The value is only used to initialize the tab 
width for the TextLayout when needed.)
TextLayout interface gets a new method:  boolean setTabWidth(int spaces)
TextLayout gets a new constant: DEFAULT_TAB_WIDTH = 8;
PrismTextLayout implements the new setTabWidth API.

I’m not sure that the Text node needs this new property.  I figured it would be 
rarely used on that class, so I had implemented it via an added property in the 
private TextAttributes class.  Maybe it isn’t needed at all.

What’s the next step?

Regards,

Scott

Reply via email to