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? 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 >>>> >>>> >>>> >>>