Re: [xwiki-devs] Proposal for Macro inline edition

2018-10-09 Thread Thomas Mortagne
On Tue, Oct 9, 2018 at 1:53 PM Marius Dumitru Florea
 wrote:
>
> On Mon, Oct 8, 2018 at 11:20 AM Thomas Mortagne 
> wrote:
>
> > On Fri, Oct 5, 2018 at 6:05 PM Marius Dumitru Florea
> >  wrote:
> > >
> > > Thomas, I'm looking at how the editor can save the macro content edited
> > > in-line and there are two options I think:
> > >
> > > (1) The editor does a separate request to a dedicated service to convert
> > > the macro content from HTML, before the entire page content is saved
> > > (2) The editor marks the macro calls that need to be converted and the
> > > conversion is done when the entire page content is converted
> > >
> > > I think the second option makes more sense. We already convert the page
> > > content from HTML to wiki syntax. We could also convert the macro content
> > > in the same flow, if needed.
> > >
> > > Note that we can't do the conversion of the macro content every time the
> > > page content is converted. We need to convert the macro content from HTML
> > > only if the editor says that is has been edited in-line. There are at
> > least
> > > 2 cases when the conversion is not needed:
> > >
> > > * when your insert a gadget in the dashboard (it uses the same Macro
> > Insert
> > > wizard from CKEditor)
> > > * when the macro outputs in-line content, since the CKEditor doesn't
> > > properly support editing in-line widgets in-place (see my other mail)
> > >
> > > So we need away to mark the macro calls that need conversion. The macro
> > > marker supports only macro name, parameters and content. One idea is to
> > use
> > > some "reserved" parameter, such as "__requiresHTMLConversion".
> > >
> > > WDYT?
> >
> > Yes +1 for (2), that's the safest and what makes the most sense anyway.
> >
> > For the marker I'm not a big fan of the parameter but hard to find a
> > retro compatible syntax otherwise (we should have put keywords in the
> > initial annotation syntax instead of just the ordered values...).
> >
> >
>
> > Now a marker is not enough, you also need to indicate the target
> > syntax (the html parser might now know it) in which it needs to be
> > converted.
>
>
> I was thinking that the parser (or some filter before the parser is called)
> could use the SyntaxDescriptor component from
> https://github.com/xwiki/xwiki-rendering/pull/145/files to determine the
> target syntax for the macro content based on the macro parameters.
> Indicating the syntax in the macro call is OK as long as the syntax is
> static and the editor gets it from the meta data attributes. But if the
> content syntax depends on the macro parameters then the editor needs to
> make a request to compute the syntax each time the macro parameters are
> modified.

SyntaxDescriptor is useless, knowing the macro won't give you the
syntax in many cases since you need the current syntax in a given
context. But yes for very standard macros this could be found in the
context and previous metadata blocks (provided the user did not
removed a metadata block by mistake, I guess the WYSIWYG could have
some protection against that).

My point was more about future custom inline editors we talked about
for which it would be much easier if they can explicitly give the
information if there is a transformation need.

>
> Also we need something which support parameters conversion
> > too since it's planned.
>
>
> I agree.
>
>
> > Finally I think I would prefer something even
> > more collision proof.
> >
> > For example:
> >
> > [[convert:__content]]=xwiki/2.1
> > param1="toto" [[convert:param1]]="xwiki/2.1"
> >
> > or
> >
> > [[convert:xwiki/2.1:__content]]="" ("" is just ignored, probably
> > better to make the html parser support not having a value at all which
> > might already be the case, I don't remember)
> > [[convert:xwiki/2.1:param1]]="toto"
> >
>
> Both look fine (with a slightly preference towards the first option) but
> I'm not convinced we need to specify the syntax. The parser has the macro
> call so it knows the macro and the parameter values so it could determine
> the syntax.

Anyway, I though about it a bit more and I think we don't need to
introduce anything in the macro annotation: the same metadata the
WYSIWYG is using to know that a content is editable could be used by
the HTML parser to know it should convert it instead of getting it
from the annotation. And this metadata is planned to support
parameters too. It also cover future custom editors need since there
is a metadata for the syntax.

>
>
> >
> > >
> > > On Wed, Oct 3, 2018 at 6:42 PM Marius Dumitru Florea <
> > > mariusdumitru.flo...@xwiki.com> wrote:
> > >
> > > > Hi everyone,
> > > >
> > > > I've started implementing this on the WYSIWYG editor side and there are
> > > > two constraints that we need to be aware:
> > > >
> > > >
> > > > (1) Macro#supportsInlineMode() must be false
> > > >
> > > > The CKEditor handles macros as widgets
> > > > <
> > https://ckeditor.com/docs/ckeditor4/latest/guide/widget_sdk_intro.html>
> > > > and widgets can have 

Re: [xwiki-devs] Proposal for Macro inline edition

2018-10-09 Thread Marius Dumitru Florea
On Mon, Oct 8, 2018 at 11:20 AM Thomas Mortagne 
wrote:

> On Fri, Oct 5, 2018 at 6:05 PM Marius Dumitru Florea
>  wrote:
> >
> > Thomas, I'm looking at how the editor can save the macro content edited
> > in-line and there are two options I think:
> >
> > (1) The editor does a separate request to a dedicated service to convert
> > the macro content from HTML, before the entire page content is saved
> > (2) The editor marks the macro calls that need to be converted and the
> > conversion is done when the entire page content is converted
> >
> > I think the second option makes more sense. We already convert the page
> > content from HTML to wiki syntax. We could also convert the macro content
> > in the same flow, if needed.
> >
> > Note that we can't do the conversion of the macro content every time the
> > page content is converted. We need to convert the macro content from HTML
> > only if the editor says that is has been edited in-line. There are at
> least
> > 2 cases when the conversion is not needed:
> >
> > * when your insert a gadget in the dashboard (it uses the same Macro
> Insert
> > wizard from CKEditor)
> > * when the macro outputs in-line content, since the CKEditor doesn't
> > properly support editing in-line widgets in-place (see my other mail)
> >
> > So we need away to mark the macro calls that need conversion. The macro
> > marker supports only macro name, parameters and content. One idea is to
> use
> > some "reserved" parameter, such as "__requiresHTMLConversion".
> >
> > WDYT?
>
> Yes +1 for (2), that's the safest and what makes the most sense anyway.
>
> For the marker I'm not a big fan of the parameter but hard to find a
> retro compatible syntax otherwise (we should have put keywords in the
> initial annotation syntax instead of just the ordered values...).
>
>

> Now a marker is not enough, you also need to indicate the target
> syntax (the html parser might now know it) in which it needs to be
> converted.


I was thinking that the parser (or some filter before the parser is called)
could use the SyntaxDescriptor component from
https://github.com/xwiki/xwiki-rendering/pull/145/files to determine the
target syntax for the macro content based on the macro parameters.
Indicating the syntax in the macro call is OK as long as the syntax is
static and the editor gets it from the meta data attributes. But if the
content syntax depends on the macro parameters then the editor needs to
make a request to compute the syntax each time the macro parameters are
modified.

Also we need something which support parameters conversion
> too since it's planned.


I agree.


> Finally I think I would prefer something even
> more collision proof.
>
> For example:
>
> [[convert:__content]]=xwiki/2.1
> param1="toto" [[convert:param1]]="xwiki/2.1"
>
> or
>
> [[convert:xwiki/2.1:__content]]="" ("" is just ignored, probably
> better to make the html parser support not having a value at all which
> might already be the case, I don't remember)
> [[convert:xwiki/2.1:param1]]="toto"
>

Both look fine (with a slightly preference towards the first option) but
I'm not convinced we need to specify the syntax. The parser has the macro
call so it knows the macro and the parameter values so it could determine
the syntax.


>
> >
> > On Wed, Oct 3, 2018 at 6:42 PM Marius Dumitru Florea <
> > mariusdumitru.flo...@xwiki.com> wrote:
> >
> > > Hi everyone,
> > >
> > > I've started implementing this on the WYSIWYG editor side and there are
> > > two constraints that we need to be aware:
> > >
> > >
> > > (1) Macro#supportsInlineMode() must be false
> > >
> > > The CKEditor handles macros as widgets
> > > <
> https://ckeditor.com/docs/ckeditor4/latest/guide/widget_sdk_intro.html>
> > > and widgets can have nested editable parts, which is great, BUT these
> > > nested editable parts must have a block-level element as parent. The
> list
> > > of elements that are accepted as in place editing hosts can be found
> here
> > >
> https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_dtd.html#property-S-editable
> > > . This means that only block-level widgets can be edited in place. As a
> > > consequence, only block-level macros can be edited in place. There's an
> > > open issue about this
> https://github.com/ckeditor/ckeditor-dev/issues/1091
> > > but I can't tell if it's going to be fixed soon or not.
> > >
> > > What options do we have?
> > >
> > > (a) enable in place editing only for macros that have
> supportsInlineMode
> > > false; that's the easiest but it excludes from the start macros such as
> > > info, error, warning, which is a pity.
> > > (b) activate in place editing only when the macro generates block level
> > > content; this means that the users will be able to edit in place a
> warning
> > > macro that is standalone, such as:
> > >
> > > -8<-
> > > before
> > >
> > > {{warning}}message{{/warning}}
> > >
> > > after
> > > ->8-
> > >
> > > but not a warning macro that is inside some paragraph