[TYPO3-mvc] textareaRTE viewhelper

[Bastian Waidelich]

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?). 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".

- 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? ;)

- 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. To solve this, we might add an intermediate 
class RteConfiguration that contains the required public fields.

- 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)


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/