Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XWIKI-22165: Home page icons do not have a text alternative #3123

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.xwiki.properties.annotation.PropertyDescription;
import org.xwiki.properties.annotation.PropertyMandatory;
import org.xwiki.properties.annotation.PropertyName;
import org.xwiki.stability.Unstable;

/**
* Parameters for the {@link DisplayIconMacro} Macro.
Expand All @@ -38,6 +39,8 @@ public class DisplayIconMacroParameters

private String iconSet;

private String textAlternative;

private boolean fallback = true;

/**
Expand Down Expand Up @@ -98,4 +101,26 @@ public void setFallback(boolean fallback)
{
this.fallback = fallback;
}

/**
* @since 16.10.0
* @return the text alternative picked for the icon
*/
@Unstable
public String getTextAlternative()
{
return this.textAlternative;
}

/**
* @since 16.10.0
* @param textAlternative a text alternative for the icon
*/
@PropertyName("Text Alternative")
@PropertyDescription("A text alternative for this icon.")
@Unstable
public void setTextAlternative(String textAlternative)
{
this.textAlternative = textAlternative;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Sereza7 have you tried to build this module with the quality profile? It's an API it seems, and you're missing unstable annotations I think. And I'm surprised revapi is not complaining at all.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding methods to a class isn't breaking anything, so I wouldn't expect any errors.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, not an interface. Still missing the unstable annotations then.

Copy link
Contributor Author

@Sereza7 Sereza7 Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the quality profile, I hit the ClassFanOutComplexity on DisplayIconMacro, working on fixing it...

Copy link
Contributor Author

@Sereza7 Sereza7 Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the Unstable annotation in 7d12a73 👍
Also reduced the fanout complexity by choosing to use a paragraph block instead of a FormatBlock for the text alternative. Now passes mvn clean install -f xwiki-platform-core/xwiki-platform-icon/xwiki-platform-icon-macro -Pquality successfully.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry but using a paragraph block for the text alternative is not okay, you cannot nest a paragraph inside another paragraph and the icon macro needs to be usable in a paragraph.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted this change of block nature in 51edb3b .
Instead of this, I set some methods in their own class in order to reduce the ClassFanOutComplexity of DisplayIconMacro.

The PR with these new changes pass quality tests :)
mvn clean install -f xwiki-platform-core/xwiki-platform-icon/xwiki-platform-icon-macro -Pquality

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went with a solution without components since those methods are quite specific and I don't think they'd need to be reused in another context.

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
*/
package org.xwiki.icon.macro.internal;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Set;

import javax.inject.Inject;
Expand All @@ -30,7 +30,6 @@
import org.xwiki.bridge.DocumentModelBridge;
import org.xwiki.bridge.internal.DocumentContextExecutor;
import org.xwiki.component.annotation.Component;
import org.xwiki.icon.IconException;
import org.xwiki.icon.IconRenderer;
import org.xwiki.icon.IconSet;
import org.xwiki.icon.IconSetManager;
Expand All @@ -40,14 +39,12 @@
import org.xwiki.rendering.async.internal.AbstractExecutedContentMacro;
import org.xwiki.rendering.async.internal.block.BlockAsyncRendererConfiguration;
import org.xwiki.rendering.block.Block;
import org.xwiki.rendering.block.ParagraphBlock;
import org.xwiki.rendering.block.FormatBlock;
import org.xwiki.rendering.block.WordBlock;
import org.xwiki.rendering.block.XDOM;
import org.xwiki.rendering.listener.MetaData;
import org.xwiki.rendering.macro.MacroExecutionException;
import org.xwiki.rendering.syntax.Syntax;
import org.xwiki.rendering.transformation.MacroTransformationContext;
import org.xwiki.security.authorization.ContextualAuthorizationManager;
import org.xwiki.security.authorization.Right;
import org.xwiki.user.UserReferenceSerializer;

/**
Expand Down Expand Up @@ -82,6 +79,10 @@ public class DisplayIconMacro extends AbstractExecutedContentMacro<DisplayIconMa

@Inject
private DocumentContextExecutor documentContextExecutor;

private final IconParser iconParser = new IconParser();

private final IconSetRetriever iconSetRetriever = new IconSetRetriever();

/**
* Default constructor.
Expand All @@ -100,12 +101,14 @@ public List<Block> execute(DisplayIconMacroParameters parameters, String content
List<Block> result;

try {
IconSet iconSet = getIconSet(parameters);
IconSet iconSet = iconSetRetriever.getIconSet(parameters, this.iconSetManager, this.documentAccessBridge,
this.contextualAuthorization);

if (iconSet == null) {
result = List.of();
} else {
XDOM iconBlock = parseIcon(parameters, context, iconSet);
XDOM iconBlock = iconParser.parseIcon(parameters, context, iconSet,
this.parser, this.defaultEntityReferenceSerializer, this.iconRenderer);

BlockAsyncRendererConfiguration rendererConfiguration =
createBlockAsyncRendererConfiguration(null, iconBlock, null, context);
Expand Down Expand Up @@ -142,59 +145,16 @@ public List<Block> execute(DisplayIconMacroParameters parameters, String content
throw new MacroExecutionException("Failed parsing and executing the icon.", e);
}

return result;
}

private XDOM parseIcon(DisplayIconMacroParameters parameters, MacroTransformationContext context, IconSet iconSet)
throws IconException, MacroExecutionException
{
String iconContent = this.iconRenderer.render(parameters.getName(), iconSet);
MetaData metaData = null;

if (iconSet.getSourceDocumentReference() != null) {
String stringReference =
this.defaultEntityReferenceSerializer.serialize(iconSet.getSourceDocumentReference());
metaData = new MetaData(Map.of(MetaData.SOURCE, stringReference));
}

XDOM iconXDOM = this.parser.parse(iconContent, Syntax.XWIKI_2_1, context, false, metaData, true);
if (!context.isInline()) {
// Wrap the children of the XDOM in a paragraph. We don't ask the parser to produce block content as
// icons should always be inline, and some icons are defined as raw inline HTML.
Block wrapper = new ParagraphBlock(iconXDOM.getChildren());
iconXDOM.setChildren(List.of(wrapper));
}
return iconXDOM;
}

private IconSet getIconSet(DisplayIconMacroParameters parameters) throws IconException, MacroExecutionException
{
IconSet iconSet;

if (parameters.getIconSet() == null) {
iconSet = this.iconSetManager.getCurrentIconSet();
} else {
iconSet = this.iconSetManager.getIconSet(parameters.getIconSet());
}

// Check if the current user can access the icon theme. If not, fall back to the default icon theme or throw
// an exception when the fallback is disabled.
if (iconSet != null && iconSet.getSourceDocumentReference() != null
&& !this.contextualAuthorization.hasAccess(Right.VIEW, iconSet.getSourceDocumentReference()))
{
if (parameters.isFallback()) {
iconSet = null;
} else {
throw new MacroExecutionException(
String.format("Current user [%s] doesn't have view rights on the icon set's document [%s]",
this.documentAccessBridge.getCurrentUserReference(), iconSet.getSourceDocumentReference()));
}
if (parameters.getTextAlternative() != null) {
// We complete the icon with a text alternative for screen readers.
Block textAltBlock = new FormatBlock();
textAltBlock.addChild(new WordBlock(parameters.getTextAlternative()));
textAltBlock.setParameter("class", "sr-only");
ArrayList<Block> updatedList = new ArrayList<>(result);
updatedList.add(textAltBlock);
result = List.copyOf(updatedList);
}

if (parameters.isFallback() && (iconSet == null || !iconSet.hasIcon(parameters.getName()))) {
iconSet = this.iconSetManager.getDefaultIconSet();
}

return iconSet;
return result;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* See the NOTICE file distributed with this work for additional
* information regarding copyright ownership.
*
* This is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as
* published by the Free Software Foundation; either version 2.1 of
* the License, or (at your option) any later version.
*
* This software is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this software; if not, write to the Free
* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
*/
package org.xwiki.icon.macro.internal;

import org.xwiki.icon.IconException;
import org.xwiki.icon.IconRenderer;
import org.xwiki.icon.IconSet;
import org.xwiki.icon.macro.DisplayIconMacroParameters;
import org.xwiki.model.reference.EntityReferenceSerializer;
import org.xwiki.rendering.block.Block;
import org.xwiki.rendering.block.ParagraphBlock;
import org.xwiki.rendering.block.XDOM;
import org.xwiki.rendering.listener.MetaData;
import org.xwiki.rendering.macro.MacroContentParser;
import org.xwiki.rendering.macro.MacroExecutionException;
import org.xwiki.rendering.syntax.Syntax;
import org.xwiki.rendering.transformation.MacroTransformationContext;

import java.util.List;
import java.util.Map;

class IconParser
{
public XDOM parseIcon(DisplayIconMacroParameters parameters, MacroTransformationContext context,
IconSet iconSet, MacroContentParser parser,
EntityReferenceSerializer<String> defaultEntityReferenceSerializer,
IconRenderer iconRenderer)
throws IconException, MacroExecutionException
{
String iconContent = iconRenderer.render(parameters.getName(), iconSet);
MetaData metaData = null;

if (iconSet.getSourceDocumentReference() != null) {
String stringReference = defaultEntityReferenceSerializer.serialize(iconSet.getSourceDocumentReference());
metaData = new MetaData(Map.of(MetaData.SOURCE, stringReference));
}

XDOM iconXDOM = parser.parse(iconContent, Syntax.XWIKI_2_1, context, false, metaData, true);
if (!context.isInline()) {
// Wrap the children of the XDOM in a paragraph. We don't ask the parser to produce block content as
// icons should always be inline, and some icons are defined as raw inline HTML.
Block wrapper = new ParagraphBlock(iconXDOM.getChildren());
iconXDOM.setChildren(List.of(wrapper));
}
return iconXDOM;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* See the NOTICE file distributed with this work for additional
* information regarding copyright ownership.
*
* This is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as
* published by the Free Software Foundation; either version 2.1 of
* the License, or (at your option) any later version.
*
* This software is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this software; if not, write to the Free
* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
*/
package org.xwiki.icon.macro.internal;

import org.xwiki.bridge.DocumentAccessBridge;
import org.xwiki.icon.IconException;
import org.xwiki.icon.IconSet;
import org.xwiki.icon.IconSetManager;
import org.xwiki.icon.macro.DisplayIconMacroParameters;
import org.xwiki.rendering.macro.MacroExecutionException;
import org.xwiki.security.authorization.ContextualAuthorizationManager;
import org.xwiki.security.authorization.Right;


class IconSetRetriever
{
public IconSet getIconSet(DisplayIconMacroParameters parameters,
IconSetManager iconSetManager, DocumentAccessBridge documentAccessBridge,
ContextualAuthorizationManager contextualAuthorization)
throws IconException, MacroExecutionException
{
IconSet iconSet;

if (parameters.getIconSet() == null) {
iconSet = iconSetManager.getCurrentIconSet();
} else {
iconSet = iconSetManager.getIconSet(parameters.getIconSet());
}

// Check if the current user can access the icon theme. If not, fall back to the default icon theme or throw
// an exception when the fallback is disabled.
if (iconSet != null && iconSet.getSourceDocumentReference() != null
&& !contextualAuthorization.hasAccess(Right.VIEW, iconSet.getSourceDocumentReference()))
{
if (parameters.isFallback()) {
iconSet = null;
} else {
throw new MacroExecutionException(
String.format("Current user [%s] doesn't have view rights on the icon set's document [%s]",
documentAccessBridge.getCurrentUserReference(), iconSet.getSourceDocumentReference()));
}
}

if (parameters.isFallback() && (iconSet == null || !iconSet.hasIcon(parameters.getName()))) {
iconSet = iconSetManager.getDefaultIconSet();
}

return iconSet;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
.runTransformations
.#-----------------------------------------------------
.input|xwiki/2.1
.# Verify the text alternative functionality
.#-----------------------------------------------------
{{displayIcon name="home" textAlternative="Home" /}}
.#-----------------------------------------------------
.expect|event/1.0
.#-----------------------------------------------------
beginDocument
beginMacroMarkerStandalone [displayIcon] [name=home|textAlternative=Home]
beginMetaData [[syntax]=[XWiki 2.1]]
beginParagraph
beginFormat [NONE] [[class]=[icon][data-xwiki-icon]=[homeIcon]]
onWord [i]
endFormat [NONE] [[class]=[icon][data-xwiki-icon]=[homeIcon]]
endParagraph
endMetaData [[syntax]=[XWiki 2.1]]
beginFormat [NONE] [[class]=[sr-only]]
onWord [Home]
endFormat [NONE] [[class]=[sr-only]]
endMacroMarkerStandalone [displayIcon] [name=home|textAlternative=Home]
endDocument
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,13 @@ XWiki can be used as a knowledge base (support, documentation, sales, etc.), for

To make the most out of your wiki, log-in and:

Use the {{displayIcon name="pencil"/}} button above to //edit// this page and start customizing your wiki to your needs.
Use the {{displayIcon name="pencil" textAlternative="Edit"/}} button above to //edit// this page and start customizing your wiki to your needs.

Use the {{displayIcon name="add"/}} button above to //add// more pages to your wiki and create the //hierarchy// that best organizes your content.
Use the {{displayIcon name="add" textAlternative="Create"/}} button above to //add// more pages to your wiki and create the //hierarchy// that best organizes your content.

Use the {{displayIcon name="home"/}} breadcrumbs located above the title to //navigate// inside your pages. It's easy to get lost in a big wiki without them.
Use the {{displayIcon name="home" textAlternative="Home"/}} breadcrumbs located above the title to //navigate// inside your pages. It's easy to get lost in a big wiki without them.

{{html}}&lt;p class="sr-only"&gt;Those three buttons are accessible using a keyboard between the global navbar elements and the main page content.&lt;/p&gt;{{/html}}

You can also use the [[Sandbox&gt;&gt;Sandbox.WebHome]] for more demo content and generally a place to experiment with your wiki's features.

Expand Down