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 &nbsp;.  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
>>>> 
>>>> 
>>>> 
>>> 

Reply via email to