[TYPO3-mvc] textareaRTE viewhelper

[Frank Frewer]

Hi Bastian,

first: thank you very much for this detailed feedback.

Bastian Waidelich schrieb:
> Frank Frewer wrote:
> 
> Hi Frank,
> 
>> today I created a suggestion for a RteViewHelper on Forge /
>> Fluid-ViewHelper-Incubator: http://forge.typo3.org/issues/9179
>> There I attached a file RteViewHelper.php and a patch for the
>> FormViewHelper.
> 
> I had a quick look at your ViewHelper and it makes a good impression! 
> Thanks for sharing!
> There are a few issues though, that prevent it to be included to the 
> core ViewHelpers:
> 
> - First, and most severe, it would be a shame if we had to change the 
> form VH in order to support the RTE VH. Especially binding it explicitly 
> to the RTE implementation (btw: would that work with multiple RTEs in 
> the same form at all? 

Yes.

).Instead we might think about a more general way
> to append/prepend JavaScript to a form. But in this case it could even 
> work without modifying the Form by adding the JS and onsubmit handler 
> "unobtrusively".

I was thinking about that, but I didn't find a way to do it. For sure 
it's better not to bind it explicitly to the RTE implementation.  The 
way could be to create a general viewHelperVariableContainer which holds
the JavaScripts/the onsubmit handler for each Formfield VH which wants 
to add some code and to create a method or some methods which put it all 
together.
But I don't know e.g. how to change the onsubmit attribute of the form 
outside the Form VH.
Maybe it could be a way to wrap the Form VH with a FormJavascript VH to 
append/prepend Javascript:
<f:formjavascript>
	<f:form  ...>
	</f:form>
</f:formjavascript>

> 
> - We don't have a written CGL for Extbase yet, but we're kind of 
> following the FLOW3 CGL [1]. That means, you shouldn't use underscores 
> and all-uppercase-abbreviation for example. And we're trying to reduce 
> the use of abbreviations in general. The goal is to know the intention 
> of a variable/method by reading its name:
> $additionalJS_submit_complete could be $additionalJavaScriptOnSubmit
> $RTEObj => $RteObject (we can assume that the reader know that Rte stays 
> for RichTextEditor here)
> $PA => ...uhm, what is it? ;)

Thanks for this hints. I will change that.

> 
> - You shouldn't use public properties in the ViewHelper. You probably 
> added those, cause tx_rtehtmlarea_pi2::drawRTE() expects an object with 
> those public properties. 

Exactly.

> To solve this, we might add an intermediate 
> class RteConfiguration that contains the required public fields.

I will try that.

> 
> - Your render method is pretty large. Maybe the rendering of the Rte 
> could be moved into a separate protected function. Besides you could add 
> helper methods like wrapJavaScriptTags (btw: they should also add CDATA 
> tags around the JS content)

I will try that too. In case of the RTE the CDATA tags comes with public 
properties $this->additionalJS_pre / $this->additionalJS_post

> 
> 
> This might sound like splitting hairs.. But there are three (and 
> probably more) simple reasons:
> 
> 1. Readability (by adding meaningful names and separating logical blocks)
> 
> 2. Extensibility (by splitting up the functionality into multiple 
> methods, one can easily extend the VH and override only the needed parts)
> 
> 3. Testability (with the current version it would be very hard to write 
> unit tests for the RTE VH. But we need those for core classes)
> 
> Best,
> Bastian
> 
> 
> 
> [1] 
> http://flow3.typo3.org/documentation/coding-guidelines//doc_core_cgl/4.4.1/view/ 

This link shows an error.

Regards,

Frank