XWIKI-21452: Macros info, success, warning and error are only distinguished by colors #3023

Sereza7 · 2024-03-27T13:01:35Z

Jira URL

https://jira.xwiki.org/browse/XWIKI-21452

Changes

Description

Updated style to take into account the addition of an icon in xwiki-rendering
Fixed tests

Clarifications

Most of these changes were initially merged with #2590, but after a passage through CI we noticed a lot of tests broke because of the changes. So we reverted it and I updated the failing tests to fit the new architecture of the message macros.

Screenshots & Video

Executed Tests

First, I built all changes with :

mvn clean install -f xwiki-rendering-macros/xwiki-rendering-macro-message -Pquality,integration-tests,docker;mvn clean install -f xwiki-rendering-macros/xwiki-rendering-macro-box -Pquality,integration-tests,docker; mvn clean install -f xwiki-rendering-api -Pdocker,integration-tests,quality; spd-say "DONE"

mvn clean install -f xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-xwiki -Pdocker,quality,integration-tests; mvn clean install -f xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-transformations/xwiki-platform-rendering-transformation-icon -Pdocker,integration-tests,quality;spd-say "DONE"

and

mvn clean install -f xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-resources

Then I checked all places that failed on the CI when we merged #2590:
platform-panels-test-docker checked on 18/03 17:00
platform-administration-test-docker checked on 25/03
platform-invitation-test-docker checked on 18/03 14:50
platform-livedata-test-docker checked on 18/03 14:40
platform-flamingo-skin-test-docker checked on 21/03 15:00
platform-realtime-wysiwyg-test-docker checked on 18/03 14:10
platform-extension-test-tests checked on 18/03 morning
platform-repository-test-tests checked on 18/03 morning
platform-rendering-xwiki checked on 18/03 13h40

I tested all of those with a command similar to:

mvn clean install -f xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-xwiki -Pdocker,quality,integration-tests`

All of those needed a slight change to the tests themselves. Most of the changes only needed to take into account the new layout inside the blocks, which ended up adding a few newlines in the computed text.
Administration tests were a bit complex to fix, because some cases used the #info velocity template while others used the {{info}} XWiki macro. In the end I added a formatting step when retrieving the content of the block that changed.

Expected merging strategy

Prefers squash: Yes
Backport on branches:
- No backport. Dangerous and not critical.
Should only be merged after XRENDERING-736: Macros info, success, warning and error are only distinguished by colors xwiki-rendering#296

…uished by colors * Updated style to take into account the addition of an icon in xwiki-rendering

…uished by colors * Updated style to differentiate the title from the content of the box

…uished by colors * Updated style to align text in the box with a grid. Note: we don't know the exact height we want for this box (depends on content), so we can't use a column flexbox.

…uished by colors * Cleaned up style

…uished by colors * Updated padding and icon position for a cleaner presentation

…uished by colors * Fixed width of the boxes fitted to content and icon alignement

…uished by colors * Fixed presentation for inplace editing

…uished by colors * Implementing the IconProvider interface to override xwiki-rendering behavior TODO: use a similar code to the icon nacro to provide the raw content of the block

…uished by colors * Implemented an alternative to isntantiate icons in rendering * Updated the POM * Fixed the components.txt file Note: I decided to not factorize witht the code in the DisplayIconMacro because we don't need anything related to a parameter.

…uished by colors * Updated style to better match the previous proposal

…uished by colors * Fixed codestyle and formatting

…uished by colors * Added mapping for the transform icons to fontAwesome

…uished by colors * Reverted approximate mapping for the transform icons to fontAwesome

…uished by colors * Increased component priority

…uished by colors * Started reflecting changes from rendering

…uished by colors * Updated position of the IconProvider to match the changes in rendering * Updated the return type of the iconProvider

* Updated the XWikiIconProvider class to fit the changes on `platform`.

…uished by colors * Updated the position of the XWikiIconProvider to reflect the move in `xwiki-rendering`. Now in the `util` package instead of being in the `transformation` package.

…uished by colors * Updated the `since` value

…uished by colors * in the inline version, reduced the white space between the border and the icon

…uished by colors * Added a text alternative to the icons provided for boxes * Added default translations for the box icons. * Updated release numbers for master branch Note: This does not have any visual effect, but facilitates things for screen readers.

…uished by colors * Changed margins and gaps between icon and borders * Removed borders from the message macro * Removed box shadow from the message macro * Added a visually distinguished left border to each type of message

…uished by colors * Factorized some style * Added a space on the line `border: none`

…uished by colors * Fixed various test checks to fit the new message style.

…uished by colors * Passes `mvn clean install -f xwiki-platform-core/xwiki-platform-invitation/xwiki-platform-invitation-test -Pquality,docker,integration-tests`

…uished by colors * Fixed some tests

…uished by colors * Fixed style for inline boxes

…uished by colors * Updated version number

tmortagne · 2024-08-06T13:25:18Z

...tform-rendering-xwiki/src/main/java/org/xwiki/rendering/internal/util/XWikiIconProvider.java

+    @Inject
+    private IconRenderer iconRenderer;
+    @Inject
+    private ContextualLocalizationManager l10n;


You don't seems to be using this component.

Fixed in 4a8cecb 👍
Thanks

tmortagne · 2024-08-06T13:28:31Z

...ssage/src/main/java/org/xwiki/rendering/internal/macro/message/XWikiWarningMessageMacro.java

+@Singleton
+public class XWikiWarningMessageMacro extends WarningMessageMacro
+{
+    private static final String ICON_PRETTY_NAME_KEY = "rendering.macro.message.icon.alternative.warning";


So you decided to make this pretty name concept related to the macro and not to the icon chosen for the macro (i.e. two macro might have the same and icon show a different pretty name), right ?

We decided in https://forum.xwiki.org/t/accessibility-html-codestyle-icons-along-text-alternatives/13378 that icons should always be used next to a text alternative. This is why I pushed for having the pretty names independent of the icons and to not provide them by the icon manager.

Sure, I just wanted to be sure it was the intended goal given the handling of the translations.

Yeah, because the alternative of the icon is context dependant. Eg. what is used as a success icon in the SuccessMessageMacro could also be a done icon in another context.

tmortagne · 2024-08-06T13:43:47Z

I didn't implement the mini translation service because it adds a lot of architecture (and need for tests :))) ) for only four translations.

The minimum is not really a "translation service" (and certainly not a public one). All you need is an internal macro icon pretty name provider which is overwritten on platform. It's a pity to introduce an icon provider to avoid overwriting those macros to finally overwrite all those macros for the icon pretty name…

…uished by colors * Removed useless component from the XWikiIconProvider.java

…uished by colors * Updated the translation of pretty name to avoid overridding macros

…uished by colors * Moved around the translations closer to where they are used.

…uished by colors * Renamed the translation keys to be more in line with the current model

Sereza7 · 2024-08-07T14:08:21Z

Ready for reviews.

…uished by colors * Updated the parameter for the iconAlternative getter

…uished by colors * Updated API doc to make clear that provided icons are always inline

tmortagne · 2024-08-08T06:19:05Z

...os/xwiki-platform-rendering-macro-message/src/main/resources/ApplicationResources.properties

@@ -0,0 +1,48 @@
+# ---------------------------------------------------------------------------


Shouldn't this file be removed ? All those translations are already provided by xwiki-rendering-macro-message.

Addressed in 2df9eec 👍

…uished by colors * Updated API doc to make clear that provided icons are always inline

tmortagne · 2024-08-09T09:01:39Z

...ge/src/main/java/org/xwiki/rendering/internal/macro/message/MacroIconPrettyNameProvider.java

+ */
+@Component(roles = MacroIconPrettyNameProvider.class)
+@Singleton
+public class MacroIconPrettyNameProvider


This does not override anything (it just conflict with the rendering class at classloader lever, so one or the other is randomly used), it should have extended the xwiki-rendering MacroIconPrettyNameProvider (and be named XWikiMacroIconPrettyNameProvider to be more clear).

Fixed in eb30e70 👍 Thank you

xwiki-platform-distribution/xwiki-platform-distribution-war-dependencies/pom.xml

tmortagne · 2024-08-09T09:03:22Z

Sorry, I missed some mistakes before merging (but would have been nice to build and test all that after the refactoring).

vmassol · 2024-08-19T08:53:51Z

@Sereza7 Shouldn't the following be changed too:

xwiki-platform/xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-templates/src/main/resources/templates/extension.vm

Line 742 in 80609c3

<div class="infomessage">$services.localization.render($emptyPlanMessageKeys.get($plan.jobType))</div>
xwiki-platform/xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-templates/src/main/resources/templates/distribution/macros.vm

Line 124 in bd419a5

<div class="infomessage">$services.localization.render('extensions.advancedSearch.noResults',
xwiki-platform/xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-templates/src/main/resources/templates/changesdoc.vm

Line 137 in b6080ac

<div class="infomessage">
xwiki-platform/xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-templates/src/main/resources/templates/changesdoc.vm

Line 141 in b6080ac

<div class="errormessage">

There might be other places like these. IMO they should be changed to use the #info, #warning and #error macros.

EDIT: Note that strangely

xwiki-platform/xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-templates/src/main/resources/templates/extensionHistory.vm

Line 123 in bd419a5

<div class="box infomessage">$services.icon.renderHTML('info') $message</div>

already has the "box" class

vmassol · 2024-08-19T09:00:03Z

Also, why is

xwiki-platform/xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-templates/src/main/resources/templates/macros.vm

Line 963 in beaeb18

different in structure from the error or warning ones?

tmortagne · 2024-08-19T11:41:48Z

different in structure from the error or warning ones?

It seems to be the same general structure, it's just that we don't show "Information:" (like we have "Error:" and "Warning:") in the case of an information message. It was already like this before.

vmassol · 2024-08-19T11:45:28Z

It seems to be the same general structure, it's just that we don't show "Information:" (like we have "Error:" and "Warning:") in the case of an information message. It was already like this before.

Doesn't seem very consistent to me and probably something to fix (possibly with a boolean parameter in the macro to decide to show the title or not).

vmassol · 2024-08-21T14:59:45Z

@Sereza7 and all: There's a discussion to have which is about the alternate text to use for icons. We used to use "Information" before and it's now been changed to "Info". I personally find "Information" to be better ("Info" can mean several things, see https://www.thefreedictionary.com/words-that-start-with-info ;)). Also we've said that we should try to avoid shortcuts in XWiki and be explicit in sentences.

I know that we're now using the Macro name and that the macro is called {{info}} but when a screen reader or otherwise reads it, I'd find "Information" to be more explicit.

For example I've had to fix the Invitation tests, see: 3050a0e

WDYT?

tmortagne · 2024-08-21T17:04:30Z

I know that we're now using the Macro name and that the macro is called {{info}} but when a screen reader or otherwise reads it, I'd find "Information" to be more explicit.

Actually, we don't really use the Macro name in XWiki. The macro name is just the fallback in xwiki-rendering, but on platform side (so in an XWiki instance), a translation is used, and the translation text is "Information".

For example I've had to fix the Invitation tests, see: 3050a0e

That test module is most probably missing xwiki-platform-rendering-macro-message. So the question is more if we want to have xwiki-platform-rendering-macro-message in the minimal WAR.

vmassol · 2024-08-22T07:44:25Z

@tmortagne interesting, thanks for the info.

So the question is more if we want to have xwiki-platform-rendering-macro-message in the minimal WAR.

Yes indeed (I see it's been added to the XS WAR:

xwiki-platform/xwiki-platform-distribution/xwiki-platform-distribution-war-dependencies/pom.xml

Lines 301 to 306 in 191e7fe

    
           <dependency> 
        
             <groupId>org.xwiki.platform</groupId> 
        
             <artifactId>xwiki-platform-rendering-macro-message</artifactId> 
        
             <version>${project.version}</version> 
        
             <scope>runtime</scope> 
        
           </dependency>

), so either that or we add it to WARBuilder for the functional tests.

Not easy to decide but I think if we push it to the extreme, a minimal war should not have message macros (it could have #info velocity macros for templates though).

So FTM I'll add the dep to ArtifactResolver#getDistributionDependencies() and revert my change in InvitationIT

vmassol · 2024-08-22T07:51:17Z

hmm actually I wonder if we shouldn't just change our deps on org.xwiki.rendering:xwiki-rendering-macro-message to a dep on xwiki-platform-rendering-macro-message in platform, and remove

xwiki-platform/xwiki-platform-distribution/xwiki-platform-distribution-war-dependencies/pom.xml

Lines 301 to 306 in 191e7fe

    
           <dependency> 
        
             <groupId>org.xwiki.platform</groupId> 
        
             <artifactId>xwiki-platform-rendering-macro-message</artifactId> 
        
             <version>${project.version}</version> 
        
             <scope>runtime</scope> 
        
           </dependency>

For invitation-ui, that dep is brought by administration-ui.

EDIT: For now I'll do what said above and add the dep to ArtifactResolver#getDistributionDependencies() and revert my change in InvitationIT but I'm not sure if that's the best, so let me know what you think.

Sereza7 and others added 30 commits February 19, 2024 14:27

XWIKI-21452: Macros info, success, warning and error are only disting…

e893a59

…uished by colors * Updated style to take into account the addition of an icon in xwiki-rendering

XWIKI-21452: Macros info, success, warning and error are only disting…

ce0d296

…uished by colors * Updated style to differentiate the title from the content of the box

XWIKI-21452: Macros info, success, warning and error are only disting…

a2864ea

…uished by colors * Updated style to align text in the box with a grid. Note: we don't know the exact height we want for this box (depends on content), so we can't use a column flexbox.

XWIKI-21452: Macros info, success, warning and error are only disting…

9cb73e8

…uished by colors * Cleaned up style

XWIKI-21452: Macros info, success, warning and error are only disting…

8cd50de

…uished by colors * Updated padding and icon position for a cleaner presentation

XWIKI-21452: Macros info, success, warning and error are only disting…

9b0ef83

…uished by colors * Fixed width of the boxes fitted to content and icon alignement

XWIKI-21452: Macros info, success, warning and error are only disting…

83571f1

…uished by colors * Fixed presentation for inplace editing

XWIKI-21452: Macros info, success, warning and error are only disting…

31eb18a

…uished by colors * Implementing the IconProvider interface to override xwiki-rendering behavior TODO: use a similar code to the icon nacro to provide the raw content of the block

XWIKI-21452: Macros info, success, warning and error are only disting…

ce3be8c

…uished by colors * Updated style to better match the previous proposal

XWIKI-21452: Macros info, success, warning and error are only disting…

7fa4083

…uished by colors * Fixed codestyle and formatting

XWIKI-21452: Macros info, success, warning and error are only disting…

e47c7f8

…uished by colors * Added mapping for the transform icons to fontAwesome

XWIKI-21452: Macros info, success, warning and error are only disting…

980a360

…uished by colors * Reverted approximate mapping for the transform icons to fontAwesome

XWIKI-21452: Macros info, success, warning and error are only disting…

b9393db

…uished by colors * Increased component priority

XWIKI-21452: Macros info, success, warning and error are only disting…

131db3c

…uished by colors * Started reflecting changes from rendering

XWIKI-21452: Macros info, success, warning and error are only disting…

5840570

…uished by colors * Updated position of the IconProvider to match the changes in rendering * Updated the return type of the iconProvider

XWIKI-21009: LiveData alternative to drag controls

2df7497

* Updated the XWikiIconProvider class to fit the changes on `platform`.

XWIKI-21452: Macros info, success, warning and error are only disting…

521e428

…uished by colors * Updated the position of the XWikiIconProvider to reflect the move in `xwiki-rendering`. Now in the `util` package instead of being in the `transformation` package.

XWIKI-21452: Macros info, success, warning and error are only disting…

504cd67

…uished by colors * Updated the `since` value

XWIKI-21452: Macros info, success, warning and error are only disting…

e3a0914

…uished by colors * in the inline version, reduced the white space between the border and the icon

XWIKI-21452: Macros info, success, warning and error are only disting…

c8944cc

…uished by colors * Factorized some style * Added a space on the line `border: none`

XWIKI-21452: Macros info, success, warning and error are only disting…

255e082

…uished by colors * Fixed various test checks to fit the new message style.

XWIKI-21452: Macros info, success, warning and error are only disting…

0b7dc8a

…uished by colors * Passes `mvn clean install -f xwiki-platform-core/xwiki-platform-invitation/xwiki-platform-invitation-test -Pquality,docker,integration-tests`

XWIKI-21452: Macros info, success, warning and error are only disting…

6598952

…uished by colors * Fixed some tests

Merge branch 'xwiki:master' into XWIKI-21452

448e564

Merge branch 'xwiki:master' into XWIKI-21452

9be5a4f

XWIKI-21452: Macros info, success, warning and error are only disting…

5a75166

…uished by colors * Fixed style for inline boxes

Merge branch 'xwiki:master' into XWIKI-21452

f4a1f06

Sereza7 marked this pull request as ready for review June 21, 2024 15:53

Sereza7 added 2 commits August 6, 2024 10:18

Merge branch 'xwiki:master' into XWIKI-21452

67a10e1

XWIKI-21452: Macros info, success, warning and error are only disting…

43ae07e

…uished by colors * Updated version number

tmortagne reviewed Aug 6, 2024

View reviewed changes

Sereza7 added 6 commits August 6, 2024 16:13

XWIKI-21452: Macros info, success, warning and error are only disting…

4a8cecb

…uished by colors * Removed useless component from the XWikiIconProvider.java

XWIKI-21452: Macros info, success, warning and error are only disting…

3dbf161

…uished by colors * Updated the translation of pretty name to avoid overridding macros

XWIKI-21452: Macros info, success, warning and error are only disting…

b62ecdd

…uished by colors * Updated the translation of pretty name to avoid overridding macros

XWIKI-21452: Macros info, success, warning and error are only disting…

3415d2e

…uished by colors * Updated the translation of pretty name to avoid overridding macros

XWIKI-21452: Macros info, success, warning and error are only disting…

03bc9cd

…uished by colors * Moved around the translations closer to where they are used.

XWIKI-21452: Macros info, success, warning and error are only disting…

56f6565

…uished by colors * Renamed the translation keys to be more in line with the current model

Sereza7 added 2 commits August 7, 2024 16:20

XWIKI-21452: Macros info, success, warning and error are only disting…

d4e7cca

…uished by colors * Updated the parameter for the iconAlternative getter

XWIKI-21452: Macros info, success, warning and error are only disting…

a9bf1fb

…uished by colors * Updated API doc to make clear that provided icons are always inline

tmortagne reviewed Aug 8, 2024

View reviewed changes

XWIKI-21452: Macros info, success, warning and error are only disting…

2df9eec

…uished by colors * Updated API doc to make clear that provided icons are always inline

tmortagne merged commit beaeb18 into xwiki:master Aug 8, 2024
1 check passed

tmortagne reviewed Aug 9, 2024

View reviewed changes

xwiki-platform-distribution/xwiki-platform-distribution-war-dependencies/pom.xml Show resolved Hide resolved

tmortagne reviewed Aug 9, 2024

View reviewed changes

xwiki-platform-distribution/xwiki-platform-distribution-war-dependencies/pom.xml Show resolved Hide resolved

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

XWIKI-21452: Macros info, success, warning and error are only distinguished by colors #3023

XWIKI-21452: Macros info, success, warning and error are only distinguished by colors #3023

Sereza7 commented Mar 27, 2024

tmortagne Aug 6, 2024

Sereza7 Aug 6, 2024

tmortagne Aug 6, 2024 •

edited

Loading

michitux Aug 6, 2024

tmortagne Aug 6, 2024

Sereza7 Aug 6, 2024 •

edited

Loading

tmortagne commented Aug 6, 2024 •

edited

Loading

Sereza7 commented Aug 7, 2024

tmortagne Aug 8, 2024 •

edited

Loading

Sereza7 Aug 8, 2024

tmortagne Aug 9, 2024 •

edited

Loading

Sereza7 Aug 27, 2024

tmortagne commented Aug 9, 2024 •

edited

Loading

vmassol commented Aug 19, 2024 •

edited

Loading

vmassol commented Aug 19, 2024

tmortagne commented Aug 19, 2024 •

edited

Loading

vmassol commented Aug 19, 2024

vmassol commented Aug 21, 2024

tmortagne commented Aug 21, 2024 •

edited

Loading

vmassol commented Aug 22, 2024 •

edited

Loading

vmassol commented Aug 22, 2024 •

edited

Loading

		@@ -0,0 +1,48 @@
		# ---------------------------------------------------------------------------

XWIKI-21452: Macros info, success, warning and error are only distinguished by colors #3023

XWIKI-21452: Macros info, success, warning and error are only distinguished by colors #3023

Conversation

Sereza7 commented Mar 27, 2024

Jira URL

Changes

Description

Clarifications

Screenshots & Video

Executed Tests

Expected merging strategy

tmortagne Aug 6, 2024

Choose a reason for hiding this comment

Sereza7 Aug 6, 2024

Choose a reason for hiding this comment

tmortagne Aug 6, 2024 • edited Loading

Choose a reason for hiding this comment

michitux Aug 6, 2024

Choose a reason for hiding this comment

tmortagne Aug 6, 2024

Choose a reason for hiding this comment

Sereza7 Aug 6, 2024 • edited Loading

Choose a reason for hiding this comment

tmortagne commented Aug 6, 2024 • edited Loading

Sereza7 commented Aug 7, 2024

tmortagne Aug 8, 2024 • edited Loading

Choose a reason for hiding this comment

Sereza7 Aug 8, 2024

Choose a reason for hiding this comment

tmortagne Aug 9, 2024 • edited Loading

Choose a reason for hiding this comment

Sereza7 Aug 27, 2024

Choose a reason for hiding this comment

tmortagne commented Aug 9, 2024 • edited Loading

vmassol commented Aug 19, 2024 • edited Loading

vmassol commented Aug 19, 2024

tmortagne commented Aug 19, 2024 • edited Loading

vmassol commented Aug 19, 2024

vmassol commented Aug 21, 2024

tmortagne commented Aug 21, 2024 • edited Loading

vmassol commented Aug 22, 2024 • edited Loading

vmassol commented Aug 22, 2024 • edited Loading

tmortagne Aug 6, 2024 •

edited

Loading

Sereza7 Aug 6, 2024 •

edited

Loading

tmortagne commented Aug 6, 2024 •

edited

Loading

tmortagne Aug 8, 2024 •

edited

Loading

tmortagne Aug 9, 2024 •

edited

Loading

tmortagne commented Aug 9, 2024 •

edited

Loading

vmassol commented Aug 19, 2024 •

edited

Loading

tmortagne commented Aug 19, 2024 •

edited

Loading

tmortagne commented Aug 21, 2024 •

edited

Loading

vmassol commented Aug 22, 2024 •

edited

Loading

vmassol commented Aug 22, 2024 •

edited

Loading