[TYPO3-core] RFC: #8543: Enhance page modul

[Steffen Kamper]

Hi Ingo,

"Ingo Renner" <ingo at typo3.org> schrieb im Newsbeitrag 
news:mailman.1.1212404519.12718.typo3-team-core at lists.netfielders.de...
> Hi Steffen,
>
> here's my review:
>
thanks!

> As discussed with you before, I don't see this yellow-pinkish color fit
> into the current color scheme of the BE yet. See the attached image for
> a proposal.
>
i will respect that in next patch

>
> t3lib_recordlist
> ====================
>
> * looks like the function in chunk 1 also needs a cleanup - quite hard
> to read that code
> * do not add an empty line at the end of the file (chunk 2)
>
i ever wonder how this happens because i didn't add a line there.

>
> class.tx_cms_layout.php
> ===========================
>
> * no php4 style class member declarations! (chunk 1)
>
you mean using "protected" ? Then a cleanup of the other vars should be done 
in next step.

> chunk 2:
> * do you really need a new css class? (normal-element) I guess it is
> possible to identify these divs by using css selectors already

i tried that before, but it's not easy to adress that div, so the class name 
does that job easy

> * when using a css class, you also don't need to define the column width
> in the element itself, put it in the css class
>
css doesn't allow dynamic change, that's the reason to use width inline

> chunk 3:
> * I wouldn't recommend to move the Help icon to the right of the table
> as it'll get out of sight then and make the page wider

i agree that it's outside, but i did this to be consistent, help is added 
always to the right

> * you're removing the default width of the table (480px), I'm not sure
> yet whether this could cause some trouble
>

because i switched to the fix width you can configure (and is 
preconfigured). Anyway with no width the div takes the width needed.

> chunk 4 and 6:
> * again: I wouldn't move the help to the right
>
s.o.

> chunk 7:
> * why spreading the new and edit icons to the left and right? To be
> consistent with the CEs (and most likely all other places) just move
> both of them to the left.
>
the idea was for clarity reason, but i can move them back to the left, using 
"gaps" to group icons

> chunk ?
> * the new info lay is great! Just make it as wide as the CE and let it
> show up below the row with the edit icons.
>

for simplicity i used the csh-class that exists. Problem could be if there 
are more infos shown than would fit in CE div.

> * what about this?:
> - $this->infoGif($infoArr).
> + #$this->infoGif($infoArr).
>
rubbish ;-)

>
> db_layout.php
> ===============
> * chunk 2 has leading spaces instead of tabs
>

same. My Editor add spaces when duplicating lines (bug) - i always have to 
fix that afterwards

>
> ext-cms-layout-db-layout-php.css
> ===================================
> * insert an empty line after each }
> * use lowercase hex values
>
ok

>
> template.php
> ==============
> * do not put $linkParams at the beginning of the anchor tag
>
ok
>
>
> that's it for now =)
> Ingo
>

thx for intensive review. I will take it into account after clearify of some 
points.

vg Steffen