[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: JDK-8228554: Accessibility errors in jdwp-protocol.html
From:       "serguei.spitsyn () oracle ! com" <serguei ! spitsyn () oracle ! com>
Date:       2019-08-28 16:54:33
Message-ID: af7d077a-aff7-8cce-84b9-896f9983c49c () oracle ! com
[Download RAW message or body]

Hi David,

Thank you for sharing your opinion.
I do not have any good suggestion how to improve it.

Alex, I'm fine with the fix as it is.

Thanks,
Serguei


On 8/28/19 00:23, David Holmes wrote:
> Hi Serguei,
> 
> On 28/08/2019 5:15 pm, serguei.spitsyn@oracle.com wrote:
> > Hi Alex,
> > 
> > Thank you for the update!
> > 
> > The most interesting case of a table with a multilevel indent is:
> > http://cr.openjdk.java.net/~amenkov/jdk14/jdwp-protocol_accessibility/2/jdwp-protocol.html#JDWP_EventRequest \
> >  
> > 
> > I still have a doubt as new variant looks a little less 
> > clear/comprehensive.
> > What do you think?
> 
> I think the new version looks fine - the extra cell boundary markers 
> in the original do not add anything to the clarity IMHO. I find both 
> forms equally difficult to understand.
> 
> Cheers,
> David
> 
> 
> > Thanks,
> > Serguei
> > 
> > 
> > 
> > On 8/26/19 16:44, Alex Menkov wrote:
> > > Ok.
> > > 
> > > Updated webrev:
> > > http://cr.openjdk.java.net/~amenkov/jdk14/jdwp-protocol_accessibility/webrev.2/ \
> > >  
> > > 
> > > The difference vs v.1 is:
> > > - ErrorSetNode.java - added 'style="width: 20%"' for the 1st column;
> > > - ConstantSetNode.java - fixed width of the 1st column (20% -> 30%)
> > > 
> > > generated doc:
> > > http://cr.openjdk.java.net/~amenkov/jdk14/jdwp-protocol_accessibility/2/jdwp-protocol.html \
> > >  
> > > 
> > > --alex
> > > 
> > > On 08/26/2019 16:00, David Holmes wrote:
> > > > On 27/08/2019 8:47 am, Alex Menkov wrote:
> > > > > On 08/26/2019 14:54, David Holmes wrote:
> > > > > > On 27/08/2019 3:01 am, Alex Menkov wrote:
> > > > > > > Hi Serguei,
> > > > > > > 
> > > > > > > The change is intentional - it seems to me that there were too 
> > > > > > > many borders in the struct description tables. I thought about 
> > > > > > > removing some of them (or making them thiner or changing color 
> > > > > > > to gray).
> > > > > > > I don't think absence of the lines makes comprehension of the 
> > > > > > > structures harder.
> > > > > > 
> > > > > > I like the new look - especially now we have proper headers and 
> > > > > > no more strange looking empty cells!
> > > > > > 
> > > > > > My only suggestion is to make the first column of each table the 
> > > > > > same width (were possible) so that the tables line up better - 
> > > > > > specifically the "Error Data" table's "Value" column should be 
> > > > > > the same width as the "Reply Data" table's "Type" column.
> > > > > 
> > > > > Maybe then make 1st column of "Error Data" the same width as (Type 
> > > > > + Name) columns in Out Data/Reply Data?
> > > > 
> > > > That would not look very good IMHO.
> > > > 
> > > > David
> > > > -----
> > > > 
> > > > > Then "Description" column in all tables will be 65%.
> > > > > 
> > > > > BTW just discovered at error in Constants tables - they have 
> > > > > column 20%, 5% and 65% - going to update the 2st column to be 30%
> > > > > 
> > > > > --alex
> > > > > 
> > > > > > 
> > > > > > Thanks,
> > > > > > David
> > > > > > 
> > > > > > > --alex
> > > > > > > 
> > > > > > > On 08/26/2019 00:58, serguei.spitsyn@oracle.com wrote:
> > > > > > > > Hi Alex,
> > > > > > > > 
> > > > > > > > I see one issue with new table format.
> > > > > > > > For instance look at the table for "DisposeObjects Command (14)".
> > > > > > > > Even a better example is "RedefineClasses Command (18)".
> > > > > > > > In the old tables the indentation was highlighted with the 
> > > > > > > > vertical lines.
> > > > > > > > It is missed in your version.
> > > > > > > > 
> > > > > > > > Thanks,
> > > > > > > > Serguei
> > > > > > > > 
> > > > > > > > 
> > > > > > > > On 8/23/19 16:54, Alex Menkov wrote:
> > > > > > > > > Hi all,
> > > > > > > > > 
> > > > > > > > > Please review the fix for
> > > > > > > > > https://bugs.openjdk.java.net/browse/JDK-8228554
> > > > > > > > > 
> > > > > > > > > webrev: 
> > > > > > > > > http://cr.openjdk.java.net/~amenkov/jdk14/jdwp-protocol_accessibility/webrev/ \
> > > > > > > > >  
> > > > > > > > > 
> > > > > > > > > generated docs:
> > > > > > > > > old: 
> > > > > > > > > http://cr.openjdk.java.net/~amenkov/jdk14/jdwp-protocol_accessibility/0/jdwp-protocol.html \
> > > > > > > > >  
> > > > > > > > > new: 
> > > > > > > > > http://cr.openjdk.java.net/~amenkov/jdk14/jdwp-protocol_accessibility/1/jdwp-protocol.html \
> > > > > > > > >  
> > > > > > > > > 
> > > > > > > > > specdiff:
> > > > > > > > > http://cr.openjdk.java.net/~amenkov/jdk14/jdwp-protocol_accessibility/spectdiff/diff.html \
> > > > > > > > >  
> > > > > > > > > 
> > > > > > > > > Summary:
> > > > > > > > > - "content outside of a region" issues:
> > > > > > > > > - <div role="banner"> replaced with with <header>,
> > > > > > > > > - <div role="main"> replaced with <main>;
> > > > > > > > > - table issues:
> > > > > > > > > - added column headers to all tables;
> > > > > > > > > - for every row specified row header;
> > > > > > > > > - indentation with table "colspan" reimplemented by using CSS.
> > > > > > > > > 
> > > > > > > > > --alex
> > > > > > > > 
> > 


[prev in list] [next in list] [prev in thread] [next in thread]