[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-openjfx-dev
Subject:    Re: RFR: 8130738: TextFlow's tab width is static
From:       Kevin Rushforth <kcr () openjdk ! java ! net>
Date:       2019-11-26 18:48:38
Message-ID: H5QHbLX38Nr4SJMsRrJY7PfjKULGltZaUl5qX2wHnq4=.0e3616b2-9bc8-46e0-92b8-a1a9ebe2e5ca () github ! com
[Download RAW message or body]

On Tue, 26 Nov 2019 18:40:10 GMT, Scott Palmer <swpalmer@openjdk.org> wrote:

> On Thu, 7 Nov 2019 14:56:58 GMT, Kevin Rushforth <kcr@openjdk.org> wrote:
> 
> > On Wed, 6 Nov 2019 19:11:48 GMT, Scott Palmer <swpalmer@openjdk.org> wrote:
> > 
> > > Added tabSize property to Text and TextFlow and -fx-tab-size CSS attribute to \
> > > both.  TextFlow's tab size overrides that of contained Text nodes. 
> > > ----------------
> > > 
> > > Commits:
> > > - 68d221c7: 8130738: TextFlow's tab width is static
> > > 
> > > Changes: https://git.openjdk.java.net/jfx/pull/32/files
> > > Webrev: https://webrevs.openjdk.java.net/jfx/32/webrev.00
> > > Issue: https://bugs.openjdk.java.net/browse/JDK-8130738
> > > Stats: 324 lines in 8 files changed: 260 ins; 0 del; 64 mod
> > > Patch: https://git.openjdk.java.net/jfx/pull/32.diff
> > > Fetch: git fetch https://git.openjdk.java.net/jfx pull/32/head:pull/32
> > 
> > > I should clamp so it reads back as 1 - Fixed.
> > 
> > This has implications for binding, etc., so I suspect this is not what we want. \
> > I'll look into this more when I review it. 
> > NOTE: This will need a CSR in addition to two reviewers. You can file the CSR any \
> > time, but I suggest leaving it in Draft state until Phil and I have had a first \
> > pass look at the API.
> 
> The indenting was wrong in that section and I had to make some minor changes, so I \
> corrected the bad indenting.  Kevin indicated, "In your specific case, reformatting \
> the methods in the StyleableProperties nested class that are adjacent to code you \
> add or modify seems fine, as long as you only change the indentation and not the \
> line wrapping." 
> I didn't intend to trigger an update to this pull request...I find Git awkward to \
> work with (Mercurial makes more sense to me) so I'm making mistakes as I bumble \
> around.  
> I do expect an update to address the way I've clamped to 1.  I agree with Kevin \
> that it is probable wrong. Just waiting for feedback.

I'll take a preliminary look today. I also plan to change the bug title to make it \
more clear that it is an enhancement request, since it currently reads like a bug \
report (I'll also change the CSR and PR title to match).

PR: https://git.openjdk.java.net/jfx/pull/32


[prev in list] [next in list] [prev in thread] [next in thread]