XRENDERING-764: Add an option for messages to be set as status #319
Conversation
* Created a class for the parameters specific to message macros (right now, only the new `isActive` * Used this new parameter in the macro execution to change the result accordingly. * Updated tests to fit the new method requirements.
* Fix on feature, I mixed things up earlier.
* Corrected typo
* Added the `advanced` flag to the property. This way it gets displayed with a lower priority in the CKEditor macro editor.
|
|
||
| /** | ||
| * @since 17.0.0RC1 | ||
| * @return whether or not the current message is a status. |
There was a problem hiding this comment.
Might need some more documentation on what exactly a "status" is and why would one set that to true.
|
You need to add translation keys for all those new parameters (yes, you added only one, but each macro which expose |
| /** | ||
| * @see #isStatus() | ||
| */ | ||
| private boolean isStatus; |
There was a problem hiding this comment.
When the getter is isStatus, the field name is generally expected to be status.
| * @since 17.0.0RC1 | ||
| * @param isStatus refers to {@link #isStatus()} | ||
| */ | ||
| @PropertyDescription("Whether or not this message should be announced as a status.") |
There was a problem hiding this comment.
This needs a much better description as otherwise users will be completely lost when this flag should be set. The description should clearly explain what consequences this has and when it should be used. Also, I'm wondering if it could make sense to more generically expose a "role" parameter - or is it clear that there is no other role that could be interesting?
| * | ||
| * @version $Id$ | ||
| * @since 17.0.0RC1 | ||
| */ |
There was a problem hiding this comment.
I think the whole class should be marked as @Unstable. Further, macro parameter classes normally aren't internal in XWiki.
There was a problem hiding this comment.
Yes, macro parameters being APIs, it's safer to put them under revapi scrutiny.
| throws MacroExecutionException | ||
| { | ||
| List<Block> boxFoundation = super.execute(parameters, content, context); | ||
| if (!boxFoundation.isEmpty() && getIconName() != null) { | ||
| Block defaultBox = boxFoundation.get(0); | ||
| if (parameters.isStatus()) { | ||
| defaultBox.setParameter("role", "status"); |
There was a problem hiding this comment.
Why is this parameter only used when the icon name is set?
Jira URL
https://jira.xwiki.org/browse/XRENDERING-764
Changes
Description
isActiveClarifications
moresection. We set the new property asAdvanced, so that it would only be displayed when it is set as True.Screenshots & Video
2024-12-02.16-40-15.mp4
In this video, we can see multiple uses of the info macro, with the status property set as true or left as false. Everything behaves as expected.
Executed Tests
Built the rendering module with:
mvn clean install -f xwiki-rendering-macros/xwiki-rendering-macro-message/ -Pquality.Manual tests with a local instance (see video above).
This PR should not change the default behaviour of the message macros (this added an option but actually never used this option). The only change in UI that could break some tests is the addition of a field in the CKEditor macro editor. This is the last field, so I highly doubt this would break any xpath even if there were some poorly defined ones.
Expected merging strategy