You’re right. Something I was reading indicated that while there was no default unit, ‘px’ was implicitly appended. I can’t find that now. Go figure.
Everything I find now about CSS tab-size is in agreement with what David wrote. It’s a number of spaces. With units it would be a ‘length’ but nothing supports that. So I think it makes sense to add -fx-tab-size as a CSS property, only supporting a number with no units. Scott > On Oct 17, 2019, at 2:32 PM, Phil Race <philip.r...@oracle.com> wrote: > > Really ? It defaults to pixels ? Is that just inherited as being the default > CSS unit ? > Is that FX's implementation > > Hmm. A bit of reading about web CSS says that strictly anything without an > explicit unit should be ignored. > The only exception is zero, where it doesn't matter. > > Eg see : > https://stackoverflow.com/questions/7907749/default-unit-for-font-size > > Although I am sure there are more authoratative sources than that. > > -phil. > > On 10/17/19 11:22 AM, Scott Palmer wrote: >> So do we go ahead and implement tabSize without the CSS support? I’m not >> sure the CSS property should be added before unit issues are worked out, as >> I think the units would be expected in the CSS context. E.g. CSS implicitly >> adds ‘px’ if no unit is specified. If our tabSize units are spaces, that >> breaks. >> >> Scott >> >>> On Oct 17, 2019, at 2:16 PM, Kevin Rushforth <kevin.rushfo...@oracle.com> >>> wrote: >>> >>> It might make sense to just add the tabSize property now, and later >>> consider adding a tabUnits property in the future if needed. By default, >>> having the units be "number of spaces in the current font" is what makes >>> the most sense, so before we could add tabUnits we would need to extend it >>> as you suggest. I'm not sure it's needed, though, so that would be another >>> reason not to do it now. >>> >>> It's probably best to have the type of tabSize be double rather than int. >>> We do this for most attributes to leave the door open for fractional >>> values. I don't know why anyone would want a tab that was 5.7 spaces, but >>> if we ever were to add a tabUnits property, I could easily see wanting >>> fractional values for some units. >>> >>> -- Kevin >>> >>> On 10/17/2019 10:40 AM, Scott Palmer wrote: >>>> Hi Phil, >>>> >>>> Thanks for taking a look. I was going to get back to this soon to attempt >>>> adding the CSS property as you mention in your previous email. I >>>> considered tabSize as a name as well. I don’t have a preference if we >>>> leave things as representing tabs as a number of spaces. But it wouldn’t >>>> be too difficult to support units, making it mostly compatible with CSS >>>> rules. The way I envision that is having two properties, tabSize and >>>> tabUnit. >>>> >>>> David mentioned javafx.css.SizeUnits… I looked quickly at the java docs >>>> for it, and it’s basically undocumented . So I went to >>>> https://www.w3.org/TR/css-values-3/#lengths and I see there is no CSS unit >>>> like ‘ems’ but meaning the width of a space in the current font. The >>>> problem with that is it would leave out the possibility to set the tab >>>> width to anything relative to the current implementation of 8 spaces. (In >>>> hindsight it should have been implemented based on ‘ems’, which for a >>>> fixed width font as typically used in a code editor would be the same as 8 >>>> spaces anyway) >>>> Do we add something to SizeUnits so we can work around this? e.g. ‘fxsp’ >>>> (FX-space - fx prefix to avoid a potential collision with any future >>>> official CSS units) >>>> >>>> // Two tab-related properties >>>> public void setTabSize(int size); // defaults to 8 >>>> public void setTabUnits(SizeUnits units); // ?? no unit for width of space >>>> in current font >>>> >>>> If we did the above, I would consider adding this for convenience: >>>> public void setTabWidth(int size, TabUnits units); >>>> >>>> As far as setting a range, I’m not sure there is any worthwhile benefit to >>>> enforcing a maximum. If we want to store the value in a short instead of >>>> an int to potentially save a couple bytes, sure. Otherwise, if someone >>>> wants to set tabs to be 20 spaces or 100, why should we prevent it? If >>>> there isn't a performance or memory impact, I wouldn’t enforce a maximum. >>>> >>>> Ignoring any out of range values rather than clamping seems fine to me as >>>> well. I was thinking of what happens if the value was bound to another >>>> value that strayed out of range. Clamping would get you as close as >>>> possible to the bound value, rather than stuck at the previously observed >>>> value. I just guessed that would be preferred, but if ignoring is more >>>> consistent with other values, then I agree it makes sense. As long as the >>>> behaviour is documented, I can’t think of any significant downside either >>>> way. >>>> >>>> >>>> Regards, >>>> >>>> Scott >>>> >>>> >>>>> On Oct 17, 2019, at 12:45 PM, Phil Race <philip.r...@oracle.com> wrote: >>>>> >>>>> Hi, >>>>> >>>>> Some more points which I thought about but for whatever reason did not >>>>> pen .. >>>>> >>>>> First one minor thing: tabWidth is an OK name, but it does not >>>>> conjure up that the value you specify isn't a width in pixels but the >>>>> number of discrete space characters to use. FWIW CSS calls it tab-size. >>>>> But CSS then supports pixels and ems .. that complicates matters a lot. >>>>> >>>>> https://developer.mozilla.org/en-US/docs/Web/CSS/tab-size >>>>> >>>>> Thee rest is mostly to do with "legal or sensible values" >>>>> >>>>> You have : >>>>> if (spaces < 0) >>>>> spaces = 0; >>>>> >>>>> since negative values are not something we want to support! >>>>> But I think instead of clamping you could "ignore", any out of range >>>>> value. >>>>> This is practiced elsewhere in FX where illegal values are ignored rather >>>>> than >>>>> throwing an exception, although it might be that clamping is quite common >>>>> in range cases. >>>>> >>>>> The follow on to that is that what is a sensible maximum value. >>>>> Currently we have int which is more than we need. For that matter so is >>>>> short. >>>>> I think we should have a documented and enforced sensible maximum. >>>>> Perhaps it could be "8" meaning we only support reducing tab width as I >>>>> don't know if anyone really wants to increase it. >>>>> CSS doesn't have a limit that I can see but if we are already only >>>>> supporting >>>>> it described as number of spaces, we can have this other limitation too. >>>>> If you think 8 is too small, then maybe 16 ? >>>>> Also remember we can compatibly relax it later but we can't compatibly >>>>> tighten it. >>>>> >>>>> >>>>> -phil. >>>>> >>>>> On 10/15/19 12:17 PM, Phil Race wrote: >>>>>> 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 >
