Thanks, Hannes.
More inline.
On 4/24/20 6:38 AM, Hannes Wallnoefer wrote:
Hi Jon,
+1. That’s another nice improvement in terms of HTML and CSS.
Generated docs look unchanged in Chrome, Firefox and Safari and table tabs work
fine.
In line 59 of Table.java the @code tag opens with a square bracket instead of a
curly one.
I note that some of the Table methods are never used: setStripedStyles,
setTabStyles, setRowIdPrefix. Do you foresee these methods being used in the
future?
Maybe ;-) I guess I didn't want to hard-wire the style-names. The
rowIdPrefix is the most likely one to be used; everything currently
works because we currently only have one multi-tabbed table per page,
but if that were to change, we would need this method.
We could remove the other methods (in a separate changeset) but if we
were to do so, I would want to add a bunch of explanatory comments
noting that the styles are currently fixed, but could be made mutable,
if needed, by reintroducing the methods.
I'll fix the Table {@code tag. There's a certain irony there, giving
this week's discovery in the jfr code ;-)
Hannes
Am 21.04.2020 um 22:47 schrieb Jonathan Gibbons <jonathan.gibb...@oracle.com>:
Well, that's what I get for trying to do a little bit of cleanup in the
stylesheet. I moved (without otherwise modifying) some entries, and missed an
implicit ordering issue. We definitely need a cleanup on the CSS for tables.
The (general) problem is that there are lots of custom tweaks for different
kinds of tables. Maybe we can do this incrementally, working on single or small
groups of CSS attributes in stages ... for example, getting main color,
background color, font weight, etc should all be reasonably easy (says me,
optimistically).
New webrev, api: the only change is to stylesheet.css, no *.java changes
Webrev: http://cr.openjdk.java.net/~jjg/8242649/webrev.01/
Api: http://cr.openjdk.java.net/~jjg/8242649/api.01/
-- Jon
On 4/16/20 6:20 PM, Jonathan Gibbons wrote:
OK, I will look at this tomorrow. Thanks for catching this.
-- Jon
On 4/16/20 9:39 AM, Hannes Wallnoefer wrote:
Hi Jon,
This isn’t yet a review of the code, just a quick feedback that there is a
problem with the generated API docs in Firefox and Chrome.
The first column of the overview, module, and package tables does not have the
usual alternating background colors, but the darker grey used for the table
header. This only happens in Firefox and Chrome (current versions on Mac OS),
it looks ok on Safari.
Hannes
Am 14.04.2020 um 22:08 schrieb Jonathan Gibbons <jonathan.gibb...@oracle.com>:
Please review a moderately simple change to clean up the CSS class names and
other minor details in the Table builder class.
Originally, the Table builder "just" built <table> elements. As part of the work to better
support ARIA, it was changed to create a <div> element that encloses the <table>,
and (when necessary) a <div> element for the series of tabs.
Currently, the style provided and documented as the style class for the table is actually used for the
outer <div> element, with no style class being set on the <table> itself. The primary
aspect of this change is to use a new/separate style class on the <table> element, and a standard
style class on the inner <div> for the tablist.
In addition, the construction of the caption is unified within the Table builder,
allowing us to delete an external method to create the caption. Related to that, the
caption previously contained an invisible <span> with style class tab-end and just
containing . This invisible span has now been removed.
Finally, an id for the tab panel was generated from style class for the table,
which was semantically questionable, since it assumed there would be only one
such table on any page. This is fixed by requiring an id to be set on the table
when there will be a tab panel. In future, we may want to simply put an id on
all tables created by the builder.
The "core" of the changes are in the Table class; the other changes in the src/
directory are just consequences of the changes to Table, and likewise the tests are
updated to match.
I made some minor simplifications to the stylesheet, but most of the CSS for
tables is sufficiently irregular that it warrants a separate follow-up
changeset just to clean up the CSS for tables in the main stylesheet. I also
grouped and commented the styles for tables in the HtmlStyle class.
There should be no visible changes to the generated API docs.
-- Jon
JBS: https://bugs.openjdk.java.net/browse/JDK-8242649
Webrev: http://cr.openjdk.java.net/~jjg/8242649/webrev.00/index.html
API: http://cr.openjdk.java.net/~jjg/8242649/api.00/index.html
