Hannes,
Thanks for the review.
I'd prefer to avoid using .toString() since I envisage that the internal
representation of HtmlStyle may evolve to support multiple CSS names,
and I think it would be better to keep .toString() for more "debugging"
purposes, and to retain the use of explicit fields and accessors to get
the right "flavor" of string. I'd be open to a simple encapsulation of
the cssName field as cssNames().
For the fonts issue, it may be the issue is just where the files are
located and which fonts you actually end up using in your browser. There
should be no other changes in the stylesheet, and the docs I provided
were the simple result of "make docs". But, thanks for raising the
issue; let's keep an eye on generated docs in EA builds.
-- Jon
On 3/13/20 9:48 AM, Hannes Wallnöfer wrote:
Hi Jon,
The changes look good.
What do you think about making the cssName field in HtmlStyle private and
overriding the toString() method to return it? That would make uses that rely
on string concatenation a bit simpler, and it would be harder to get the wrong
CSS class name out of it (except by explicitly using the name() method). I
guess it’s a matter of taste and it’s fine the way it is, I just wanted to
mention the possibility.
The generated docs look good to me, but the fonts look a bit bigger than my
local reference (both with and without your patch). Could it be that you have
some other change in your style sheet? Since it’s not in the patch and my
locally generated docs look the same with and without the patch I guess it’s
not a problem.
Hannes
Am 12.03.2020 um 03:12 schrieb Jonathan Gibbons <[email protected]>:
Please review a mostly-simple change to convert javadoc to using
hyphenated-naming instead of camelCase naming for CSS class names in the
stylesheet. This is a big step towards using modern naming conventions.
The change starts off in HtmlStyle.java, where the hyphenated names are
generated automatically, but with the possibility of specifying them explicitly.
There is (obviously) a corresponding change in the stylesheet, and downstream there are
changes in lots of tests. These changes were "mostly automated" by creating a
sed script to convert old-style names to new-style names. The sed script got maybe 95%
of the way there, but some manual fixup was necessary: for example, for false positive
matches.
In the source code, it was necessary to ensure that the correct form of the
name was used in the HtmlTree, and in one case, the natural (Java) name is used
explicitly to generate a JavaScript name. (Previously, all cases just used
.toString()). And, as has often recently been the case, some outlier uses of
direct construction of HtmlTree nodes is replaced by the appropriate typesafe
factory method.
Note: the end goal is to move towards more structured naming, perhaps based on
the BEM naming conventions. This change is not that, although it is a big and
necessary step in that direction. The existing naming is too irregular to
attempt to convert everything to BEM naming at this time; instead, going
forward, we can identify blocks and the corresponding groups of names and
convert them in stages, such as for summary blocks, details blocks, and
individual detail blocks. That will also be a good time to document the names
being used.
Because this potentially impacts the visual presentation, I've included a copy
of the generated JDK API docs using the new naming. The intent is that the
change should not affect the pixels on the screen, and everything should look
and work exactly as before.
-- Jon
JBS: https://bugs.openjdk.java.net/browse/JDK-8240916
Webrev: http://cr.openjdk.java.net/~jjg/8240916/webrev.00/index.html
API: http://cr.openjdk.java.net/~jjg/8240916/api.00/index.html
