[TYPO3-core] RFC: #0011499: feature: db compare in install tool can require a huge amount of clicks

[Oliver Klee]

Hi,

+1 by reading and testing. Yet, I'd love to see a reworked version that
cleans up a few things and improves usability some more:

> Index: typo3/sysext/install/mod/class.tx_install.php
> ===================================================================
>  		$content = '';
>  		switch($type)	{
>  			case 'get_form':
> -				$content = '';
> +
> +				$content.= '';

This line is a noop and can be removed.

> +				$content.= '<div style="float:right;">

Generally, please use spaces around the .= (also in the other occurences).

Please add the missing space after the : .

> +					<a href="#bottom" onclick="$(\'updateform\').select(\'input[type=checkbox]\').invoke(\'setValue\', true);">select all</a> /
> +					<a href="#bottom" onclick="$(\'updateform\').select(\'input[type=checkbox]\').invoke(\'setValue\', false);">de-select all</a>
> +					</div>	';

These should be buttons, not links (because they trigger actions and
don't lead to another page). Then you wouldn't need the dummy href either.

I'd prefer the buttons to be positioned to the left so they are nearer
to the checkboxes (which are the affected elements). -> locality principle.

I'd also love to have a pair of checkboxes beneath each checkbox section
because there often are scenarious where I eg. want to add new fields,
but don't remove any fields.

>  				$content.= $this->generateUpdateDatabaseForm_checkboxes($arr_update['clear_table'],'Clear tables (use with care!)',false,true);

Generally, please use a space after commas and make sure that the lines
are no longer than 80 characters.

>  
>  				$content.= $this->generateUpdateDatabaseForm_checkboxes($arr_update['add'],'Add fields');
> @@ -4163,6 +4169,8 @@
>  				$content.= $this->generateUpdateDatabaseForm_checkboxes($arr_remove['change_table'],'Removing tables (rename with prefix)',$this->setAllCheckBoxesByDefault,1,$arr_remove['tables_count'],1);
>  				$content.= $this->generateUpdateDatabaseForm_checkboxes($arr_remove['drop_table'],'Drop tables (really!)',$this->setAllCheckBoxesByDefault,0,$arr_remove['tables_count'],1);
>  
> +
> +
>  				$content = $this->getUpdateDbFormWrap($action_type, $content);

Maybe putting the temporary content in a local variable instead of
appending to $content and then overwriting it would improve readability. :-)


Oliver