On Mon, Oct 15, 2018 at 10:48 AM Marius Dumitru Florea <mariusdumitru.flo...@xwiki.com> wrote: > > On Tue, Oct 9, 2018 at 3:22 PM Thomas Mortagne <thomas.morta...@xwiki.com> > wrote: > > > On Tue, Oct 9, 2018 at 1:53 PM Marius Dumitru Florea > > <mariusdumitru.flo...@xwiki.com> wrote: > > > > > > On Mon, Oct 8, 2018 at 11:20 AM Thomas Mortagne < > > thomas.morta...@xwiki.com> > > > wrote: > > > > > > > On Fri, Oct 5, 2018 at 6:05 PM Marius Dumitru Florea > > > > <mariusdumitru.flo...@xwiki.com> 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="<strong>toto</strong>" [[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]]="<strong>toto</strong>" > > > > > > > > > > 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. > > > > ATM the WYSIWYG editor sends back *only* the macro markers. The macro > output is simply ignored on save because up until now all the information > that was needed was available in the macro marker so sending the macro > output was useless. It's not clear to me how the HTML parser will behave > because: > > * it will find a macro marker > * it will have to continue parsing the macro *output* in case it finds a > meta data block > * it will have to discard the parsed macro output whether it finds the meta > data block or not
If there is something the parser have to parse it in practice, you can't just jump to the end without knowing where is the end. It's just that instead of igoring everything but the end macro marker it will catch metadatas along the way to know if it should start/stop stacking events for later and know the current target syntax. Then when reaching the end macro the parser will decide what to do. If tagged content has been found, serialize the events in the target syntax otherwise just send the startmacro content as now). Same logic for the parameters when support for parameters inline editing will be added. > > Maybe the editor could send the macro markers and any meta data elements > that it finds within the macro output, discarding everything else. So > basically, on save, the editor looks for meta data elements inside the > macro output, extracts them, removes everything between the macro start / > end markers and inserts the extracted meta data elements. This way the > editor sends back only the relevant content. As you want but I don't think the WYSIWYG really need to bother. Technically it does not really change much for the parser to have everything or only the metadata except a minor optimization because we transmit less data. > > WDYT? > > Thanks, > Marius > > > > > > > > > > > > > > > > > > > > > > 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 text, such > > as: > > > > > > > > > > > > -----8<----- > > > > > > before {{warning}}message{{/warning}} after > > > > > > ----->8----- > > > > > > > > > > > > The issue is that we don't always know if the macro output is > > > > block-level > > > > > > until we render the macro so hiding the macro content text area > > when > > > > > > inserting a macro is not easy. > > > > > > > > > > > > (c) Try to implement > > > > https://github.com/ckeditor/ckeditor-dev/issues/1091 > > > > > > ourselves, but I don't think it's easy > > > > > > (d) Don't use widgets to support macros and implement something > > > > custom. I > > > > > > think this is crazy. > > > > > > > > > > > > > > > > > > (2) The second constraint is that the macro content must not be > > > > mandatory. > > > > > > > > > > > > ATM both {{info/}} and {{info}}{{/info}} generate "The required > > > > content is > > > > > > missing." and so do the other macros I've tested. So it seems > > there's > > > > no > > > > > > distinction between empty content and no content at the > > implementation > > > > > > level. Thus, if we want to hide the macro content text area when > > > > inserting > > > > > > a macro such as {{info}} then we need to make the macro content > > > > optional. > > > > > > Either by changing the macro descriptor or by changing the > > > > implementation > > > > > > to differentiate the empty content from no content specified. > > > > > > > > > > > > Thanks, > > > > > > Marius > > > > > > > > > > > > > > > > > > On Mon, Sep 10, 2018 at 2:05 PM Simon Urli <simon.u...@xwiki.com> > > > > wrote: > > > > > > > > > > > >> Hi everyone, > > > > > >> > > > > > >> I'm working on the roadmap issues related to the inline edition > > with > > > > > >> WYSIWYG editor for macro content and macro parameters. > > > > > >> > > > > > >> The first step is to add a flag to allow user specify that a > > content > > > > or > > > > > >> a parameter can be edited inline with the WYSIWYG editor. > > > > > >> > > > > > >> The second step is to allow the CKEditor to detect where the > > content > > > > > >> and/or parameters should be edited. > > > > > >> Let's take the exampe of a simple macro without any parameter, > > which > > > > > >> currently produces this code: > > > > > >> > > > > > >> <div class="box infomessage"> > > > > > >> <div class="title"> > > > > > >> <span class="icon info"></span> > > > > > >> some title > > > > > >> </div> > > > > > >> Some content > > > > > >> </div> > > > > > >> > > > > > >> We propose (me & Marius) to ask users to add a wrapper with a > > specific > > > > > >> class around the content to tell the editor it should only allow > > > > editing > > > > > >> this content, e.g.: > > > > > >> > > > > > >> <div class="box infomessage"> > > > > > >> <div class="title"> > > > > > >> <span class="icon info"></span> > > > > > >> some title > > > > > >> </div> > > > > > >> <span class="editable-content">Some content</span> > > > > > >> </div> > > > > > >> > > > > > >> About parameters, our idea was to define a new metadata attribute > > and > > > > to > > > > > >> ask users to use it for specifying the content is editable, such > > as > > > > for > > > > > >> a parameter named foo: > > > > > >> > > > > > >> <span class="editable-content" data-parameter="foo">my foo > > parameter > > > > > >> value</span> > > > > > >> > > > > > >> However I don't know right now how the editor would manage cases > > such > > > > as: > > > > > >> <span class="editable-content">Some content with <span > > > > > >> class="editable-content" data-parameter="myparameter">a > > > > > >> parameter</span></span> > > > > > >> > > > > > >> So: > > > > > >> 1. Do you agree on the usage of a class named > > "editable-content" > > > > > >> which would be used as a tag to allow inline edition? > > > > > >> 2. WDYT about using a data-parameter and this class for inline > > > > > >> editing of parameters? > > > > > >> > > > > > >> Thanks, > > > > > >> Simon > > > > > >> -- > > > > > >> Simon Urli > > > > > >> Software Engineer at XWiki SAS > > > > > >> simon.u...@xwiki.com > > > > > >> More about us at http://www.xwiki.com > > > > > >> > > > > > > > > > > > > > > > > > > > > > > -- > > > > Thomas Mortagne > > > > > > > > > > > > -- > > Thomas Mortagne > > -- Thomas Mortagne
