[XWI-142] Enable wiki macros in more editors - #24480
Conversation
Move the `wikisAvailable` check into the central CKEditor macro resolver and add a `wiki` macro preset, so any editor can opt in with a one-liner instead of repeating the logic.
The places are: - comments (activity) - custom fields (long text) - custom fields help text - meetings: - description - outcomes - agenda item
|
Warning Flaky specs
🤖 Ask Copilot to investigateCopy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer. |
NobodysNightmare
left a comment
There was a problem hiding this comment.
Overall this looks like a valid extension of the existing ICKEditorMacroType, which seems to have been intended exactly like this.
I can't say that I am a huge fan of this type and if anything, I'd urge to strengthen documentation around it and stricten type definitions up where-ever possible.
| if (resolved === 'none') { | ||
| return false; | ||
| } | ||
| if (resolved === 'resource') { |
There was a problem hiding this comment.
❓ Looking at the diff, this wasn't handled before as well, but do you know what we do for full?
|
|
||
| // Expand macro tokens and add/withhold the wiki macros based on `wikisAvailable`. | ||
| // `false`/`'none'` stay macro-free, keeping custom fields and read-only editors untouched. | ||
| private resolveMacros(macros:ICKEditorMacroType|undefined):ICKEditorMacroType|undefined { |
There was a problem hiding this comment.
I guess the return type was opened to ICKEditorMacroType|undefined, because the input type for macros was opened to |undefined as well.
Couldn't we expand the first check to
if (!resolved || resolved === 'none') {
return false;
}This would at least make our method guarantee to return a ICKEditorMacroType, regardless of input as far as I can tell.
|
|
||
| export type ICKEditorType = 'full'|'constrained'; | ||
| export type ICKEditorMacroType = 'none'|'resource'|'full'|boolean|string[]; | ||
| export type ICKEditorMacroType = 'none'|'resource'|'full'|'wiki'|boolean|string[]; |
There was a problem hiding this comment.
This is such a cursed type definition. One of four string, a boolean or a string array 🤯
Should we somewhere (here or in resolveMacros) explain what each of those strings is supposed to mean?
My current take:
- none: nothing
- resource: the resource stuff + wiki stuff
- full: I have no clue... nothing?!?
- wiki: wiki stuff only
|
|
||
| // Expand macro tokens and add/withhold the wiki macros based on `wikisAvailable`. | ||
| // `false`/`'none'` stay macro-free, keeping custom fields and read-only editors untouched. | ||
| private resolveMacros(macros:ICKEditorMacroType|undefined):ICKEditorMacroType|undefined { |
There was a problem hiding this comment.
I am even wondering whether we should split up the input type and the output type. As far as I can tell CKEditor will only digest false or a string array. The addition of single strings seems to be an extension of ours.
So maybe the input type should be what ICKEditorMacroType currently is, while the return type should be the equivalent of what CKEditor would accept? I.e. not all the simple strings?
Ticket
XWI-142
What are you trying to accomplish?
Centralise CKEditor wiki macro.
Add the wiki macro to CKEditor in many places.
The places are:
Screenshots
Merge checklist