[
https://issues.apache.org/jira/browse/DOXIA-590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17540433#comment-17540433
]
ASF GitHub Bot commented on DOXIA-590:
--------------------------------------
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).
> Either provided element class or default class gets ignored
> -----------------------------------------------------------
>
> Key: DOXIA-590
> URL: https://issues.apache.org/jira/browse/DOXIA-590
> Project: Maven Doxia
> Issue Type: Bug
> Components: Core
> Affects Versions: 1.8
> Reporter: Fred Eckertson
> Assignee: Michael Osipov
> Priority: Major
> Fix For: 2.0.0-M3, 1.11.2
>
> Attachments: image-2022-05-18-21-57-40-619.png
>
>
> The following construct is somewhat common in doxia-core
> att.addAttribute( Attribute.CLASS, "a" );
> The documentation says that basic attributes (including CLASS) are supported.
> However in cases like this either that "a" or the CLASS that was provided in
> the attributes parameter will be ignored. The correct way to do this is to
> append the provided CLASS to "a " if a CLASS attribute was provided.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
