[TYPO3-core] RFC: #9574: Configuration enhancement for indexed_search

Steffen Kamper info at sk-typo3.de
Tue Mar 3 13:51:22 CET 2009


Hi,

sry for the delay, but at the end here is the new patch taking your 
comments into account

Michael Stucki schrieb:
> 
> 1.  plugin.tx_indexedsearch.templateFile
> 
> You cannot change the value for this! If you want to change it, you need
> to use a [compatVersion] condition and write a hook for the update
> wizard to mention this.

i reverted it, so old setting is used

> 2. #what to display
>    # if results should be outputted on different page use the pid here
> 
> => Please give more care to that. Always add a space after a comment
>    mark, and fix the English (page => pages).
>

changed comment to
# what section should be displayed
# if results should be outputted on a different page, define the pid for 
the result page here

> 3. "use the pid here"
> 
> What pid? Needs to be more clear.
>
see above

> 4. plugin.tx_indexedsearch.show.advancedSearchLink
> 
> Value is changed. Must be done with a compatVersion, too.
reverted
> 
> 5. plugin.tx_indexedsearch.defaultResultsPerPage = 2
> 
> Can't be 2. The current default is 10 so unless there is a good reason
> to change it, please leave that as default.
> 
was a typo, set to 10

> Also, the name "defaultResultsPerPage" sounds odd to me. Why default? If
> you change this value, you also change the number of results per page,
> right? If so, remove the "default" from the name.
>
changed to resultsPerPage

> 6. ...pageBrowser.pageLinks_stdWrap.preCObject.dataWrap
> 
> I can't find a wrap in this value. Should this be "data" instead of
> "dataWrap"?
> 
changed to data

> 7. plugin.tx_indexedsearchbox
> 
> Should be named "tx_indexedsearch_box" (separate "box" from the rest).
>
this is not possible, as functions in t3lib_extMgm strip out any 
underscore, see
t3lib_extMgm::addPItoST43
t3lib_extMgm::addPlugin

> 
> indexed_search/locallang.xml:
> 
> 1. flexform_indexed_search.sheetTitle
> 
> Consistency: "Indexed Search" is written with capital "S" although all
> other labels use lower-case.
> 
I changed others to use upper-case "S" => Indexed Search

> 
> 
> indexed_search/pi/class.tx_indexedsearch.php:
> 
> 1.
> +205 foreach ($this->code as $c) {
> +206  switch (t3lib_div::strtolower($c)) {
> +207   case 'form':
> +208    if ($this->conf['code'] != 'form' && !$this->conf['showForm']) {
> +209     break;
> +210    }
> 
> I don't understand why the code must be "form" here. If it has a good
> reason, please add a comment to it.
>
comment added:
// if conf has only 'form' it's the plugin indexedsearch_box which needs 
the form all the time,
// when code has more components, make it dependend from the setting 
'showForm'

> 2.
> +213 case 'rules':
> +214  if ($this->conf['showRules']) {
> 
> I don't like to set "code = rules" and then rules are only displayed if
> conf.showRules is set.
> 
> Suggestion:
> - Change the default code to "defaults"
> - Consider value of conf.showRules as long as the value is "defaults"
> - Ignore conf.showRules as soon as the "code" is different to that
> 
> (applies to all other cases of this statement, too)
> 

i don't agree, here the reason:
You may use the plugin on a page, and for a condition you want to blind 
the rules. Then you can have it displayed except when the condition is true.

Also you set this in the flexform independent from the code (the CE 
indexed search always come with code=form,rules,results)

but what i did: there is the TS option plugin.tx_indexedsearch.show {} 
where rules are already in, i used that now
plugin.tx_indexedsearch.show {
   form = 1
   rules = 1
   results = 1
}

Same defaults are used in flexform.

> 3. // Browsing nav, bottom.
> 
> The line is repeated now. Most likely a typo?
> 
> 4. $pid = $this->conf['resultPid'] ? $this->conf['resultPid'] :
>                                                 $GLOBALS['TSFE']->id;
> 
> Remove $pid and set $this->conf['resultPid'] in ->initialize(), just
> like the rest.
>

done

> 
> General notes:
> 
> 1. I did not yet check the functionality of your patch. Will do so after
>    you have fixed all points mentioned above.
> 
you're welcome ;-)

> 2. I'm not quite happy that the template is changed, although I see the
>    need for it. In case someone has customized the template file and
>    points to that, the display will be broken after the update. I think
>    this should be mentioned somewhere, with a screenshot of the part
>    that must be added to that customized template.
>    Again I propose to use the update wizard for that.
>
I reverted all settings to existing, so the only part which is new is 
the part of the HTML-template.
As the manual is an extra extension in TER i raised the version and 
added a manual for update instructions and new settings which should be 
included. I attached it here.


> 3. Documentation is completely missing!!
>
see attached manual.swx

vg Steffen
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: indexed_search_pluginconfiguration_v4.diff
Url: http://lists.netfielders.de/pipermail/typo3-team-core/attachments/20090303/a757c2fa/attachment-0001.txt 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: manual.sxw
Type: application/vnd.sun.xml.writer
Size: 112558 bytes
Desc: not available
Url : http://lists.netfielders.de/pipermail/typo3-team-core/attachments/20090303/a757c2fa/attachment-0001.sxw 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: is_searchbox_plugin.png
Type: image/png
Size: 42140 bytes
Desc: not available
Url : http://lists.netfielders.de/pipermail/typo3-team-core/attachments/20090303/a757c2fa/attachment-0002.png 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: is_plugin.png
Type: image/png
Size: 47519 bytes
Desc: not available
Url : http://lists.netfielders.de/pipermail/typo3-team-core/attachments/20090303/a757c2fa/attachment-0003.png 


More information about the TYPO3-team-core mailing list