feckertson commented on code in PR #98:
URL: https://github.com/apache/maven-doxia/pull/98#discussion_r878701501
##########
doxia-core/src/main/java/org/apache/maven/doxia/sink/impl/Xhtml5BaseSink.java:
##########
@@ -1567,21 +1567,24 @@ public void tableRow()
@Override
public void tableRow( SinkEventAttributes attributes )
{
- MutableAttributeSet att = new SinkEventAttributeSet();
+ MutableAttributeSet atts = SinkUtils.filterAttributes(
+ attributes, SinkUtils.SINK_TR_ATTRIBUTES );
- if ( evenTableRow )
+ if ( atts == null )
{
- att.addAttribute( Attribute.CLASS, "a" );
+ atts = new SinkEventAttributeSet();
}
- else
+
+ String rowClass = evenTableRow ? "a" : "b";
+ if ( atts.isDefined( Attribute.CLASS.toString() ) )
{
- att.addAttribute( Attribute.CLASS, "b" );
+ String givenRowClass = (String) atts.getAttribute(
Attribute.CLASS.toString() );
+ rowClass = givenRowClass + " " + rowClass;
Review Comment:
The second option is easy to comprehend and offers ultimate flexibility,
**but** it forces a consumer to keep track of row parity just to put an
additional decoration on a single row.... not very appealing.
The first option is what I originally proposed and what has been
implemented, but it does not support a variety of use cases. Zebra striping
based on row number parity is not always correct. This is the fundamental
dilemma.
The third option blends both but adds complexity and a performance hit.
Here is a potential fourth option:
Create a new tableRow method in SinkAdapter having additional parameters and
implement the existing method to invoke it with reasonable defaults. There are
various choices for the extra parameter(s), but the following offers a lot of
flexibility while always appending the "a" or "b":
`tableRow(SinkEventAttributes attributes, boolean flipRowParity)`.
Use Cases:
- bold face just one row: pass true (or equivalently just call the original
method).
- add two rows at a time planning for only one or the other to be visible at
any one time: pass false for the first and true for the second.
- have multiple consecutive rows behave as a single row (same stripe color):
pass true for the last in each group, and false for the rest.
- start off with some rows hidden and have a toggle to show/hide them: pass
true for the displayed rows and false for the hidden ones.
`tableRow(SinkEventAttributes attributes, boolean appendStripeClass, boolean
flipRowParity)` would be even more flexible, but the only situation I see where
it makes sense to not add the "a" or "b" is if one want's to have more than two
stripe colors.
I think all the other places which apply a default class can just append it
(bodyTable, externalLink, section).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
