[#33441] fix SmartSearch menu item missing parameters by itbra · Pull Request #3285 · joomla/joomla-cms

itbra · 2014-03-12T10:41:52Z

Fixing issue described in bug #33458, related to missing menu item parameters in FinderViewSearch results page (components/com_finder/views/search/tmpl/default_results.php) not allowing to enable/disable the display of the related content.

SmartSearch evaluates 2 parameters - show_suggested_query and show_explained_query on the results page (/components/com_finder/views/search/tmpl/default_results.php) that haven't been configurable through the menu item configuration since its deployment through jXtended. But it should be configurable whether or not to suggest alternative search terms or explain how to improve the search rather than overriding the view just for this circumstance. So i added these two options to the menu item configuration file (/components/com_finder/views/search/tmpl/default.xml) and added translation to (/administrator/language/en-GB/en-GB.com_finder.sys) thus enabling the evaluation of these parameters to do what a parameter should do: enable/disable.

chrisdavenport · 2014-03-23T22:51:51Z

Could these two parameters be added to the component options too?

itbra · 2014-03-24T10:48:49Z

They are. Check the "files changed" tab. The fix covers administrator/language/en-GB/en-GB.com_finder.sys.ini and components/com_finder/views/search/tmpl/default.xml.

Or mabe i am getting you wrong and you could specify your comment, please?

chrisdavenport · 2014-03-24T10:52:55Z

I mean when you go Components -> Smart Search -> Options. I think you just need to add the fields into the config.xml file if memory serves.

itbra · 2014-03-24T10:56:18Z

I think this doesn't relate to the component config, because these options related to the view (menu item config) rather than to the component globally. Because, if for example you have more than one search page it might be wished to show them on one page while not wanting to show them on the other. A global option wouldn't allow this and make them on/off-configurable only which makes no sense to me. The menu item options instead allow for different configuration.
But i'm open for your arguments.

Bakual · 2014-03-24T12:06:17Z

@itbra What we usually do is adding the parameter in both places. In the menu item we then use an additional (default) option <option value="">JGLOBAL_USE_GLOBAL</option>.
This way you can set a behavior in the component options, and then you can override it in a menu item if needed.
The global one is especially helpful since you don't necessary have a menu item for the search.

So I'd support the request of Chris to add them to the component config plus adding the "Use Global" option to the menu item.

itbra · 2014-03-24T12:15:15Z

Your arguments make sense. So far i didn't think of setting up a global config to be overridden if required.

The global one is especially helpful since you don't necessary have a menu item for the search.

Didn't consider this. In this case it absolutely makes sense. I'll add the requested change asap.

Added *JGLOBAL_USE_GLOBAL* option to *show_suggested_query* and *show_explained_query parameters*

Added *show_suggested_query* and *show_explained_query* parameters to make them configurable and overwritable

itbra · 2014-03-24T12:41:00Z

Done. Please let me know if this suits.

chrisdavenport · 2014-03-24T19:59:31Z

administrator/components/com_finder/config.xml

Default should be "1" to correspond with current behaviour.

chrisdavenport · 2014-03-24T20:08:52Z

Good work. Just needs some tweaks to the English as indicated.

chrisdavenport · 2014-03-24T20:18:31Z

components/com_finder/views/search/tmpl/default.xml

Perhaps change the fields to type="list" for consistency with other fields on the screen?

Changed default values of *show_suggested_query* and *show_explained_query* to 1. Changed option titles from JYES and JNO to JSHOW and JHIDE

Changed translations for *COM_FINDER_SHOW_SUGGESTED_QUERY_* and *COM_FINDER_SHOW_EXPLAINED_QUERY_*

Changed field types of *show_suggested_query* and *show_explained_query* from radio to list

chrisdavenport · 2014-03-31T08:50:01Z

administrator/language/en-GB/en-GB.com_finder.sys.ini

Can you drop the question mark at the end of this sentence. My fault, I wasn't clear enough in my last comment.

chrisdavenport · 2014-03-31T08:53:10Z

With the last couple of tweaks I think it's good to go. Many thanks for working on this.

infograf768 · 2014-03-31T09:42:36Z

@chrisdavenport

I just checked both existing com.finder ini and sys.ini and found out that most lang strings in the sys.ini should be in fact in the .ini file, (including the new ones in this PR which constants should rather be of the type COM_FINDER_CONFIG_)

My intention was to correct this ASAP and adapt this PR to fit.

As far as I know, we should only keep in the sys.ini:

`; Joomla! Project
; Copyright (C) 2005 - 2014 Open Source Matters. All rights reserved.
; License GNU General Public License version 2 or later; see LICENSE.txt, see LICENSE.php
; Note : All ini files need to be saved as UTF-8

COM_FINDER="Smart Search"
COM_FINDER_XML_DESCRIPTION="Smart Search"

COM_FINDER_MENU_SEARCH_VIEW_DEFAULT_TEXT="The default search layout."
COM_FINDER_MENU_SEARCH_VIEW_DEFAULT_TITLE="Search"
`

Removed question marks

itbra · 2014-03-31T10:24:10Z

True. I was also wondering why these appear in the .sys.ini, but didn't wanna touch it since i thought there was good reason for it. Let me know if you want me to move them.

infograf768 · 2014-03-31T10:33:01Z

I can make a PR, but it would have to be applied before yours or after yours.
Whatever the decision, please change your constants to use COM_FINDER_CONFIG_ and move them alphabetically to the .ini file instead of the sys.ini

For example
+COM_FINDER_SHOW_SUGGESTED_QUERY_LABEL="Did you mean"
should be
+COM_FINDER_CONFIG_SHOW_SUGGESTED_QUERY_LABEL="Did you mean"

and evidently change in the xml(s)

My test show here that all COM_FINDER_CONFIG_etc strings in the sys.ini should be deleted.

Also the
COM_FINDER_SEARCH_FILTER_SEARCH_LABEL="Search Filter"
COM_FINDER_SEARCH_FILTER_SEARCH_DESC="Selecting a Search Filter will limit any searches submitted to use the selected filter."
COM_FINDER_SEARCH_SEARCH_QUERY_LABEL="Search Query"
COM_FINDER_SEARCH_SEARCH_QUERY_DESC="Entering search terms will make this menu item automatically return the results for the predefined terms."

Should be moved to the .ini (alpha the strings when doing such)

chrisdavenport · 2014-03-31T15:57:20Z

@infograf768 Sounds good to me.
@itbra Please make those changes if you have time. Thanks.

I've noticed some inconsistencies in the UI between the component options and the menu options, but given that we need to get 3.3 out the door, I think it best to get this PR merged and then work on a separate PR to improve the UI.

Added substring *CONFIG_* to new translation keys.

Added substring *CONFIG_* to new options

Added substring *CONFIG_* to new language keys.

itbra · 2014-04-01T10:00:34Z

Done, hope i didn't oversee anything. Please let me know.

infograf768 · 2014-04-01T10:06:03Z

You forgot to add your new ini strings to /administrator/language/en-GB/en-GB.com_finder.ini instead of the sys.ini

Also, you need to move from the sys.ini to the ini:

COM_FINDER_SEARCH_FILTER_SEARCH_LABEL="Search Filter"
COM_FINDER_SEARCH_FILTER_SEARCH_DESC="Selecting a Search Filter will limit any searches submitted to use the selected filter."
COM_FINDER_SEARCH_SEARCH_QUERY_LABEL="Search Query"
COM_FINDER_SEARCH_SEARCH_QUERY_DESC="Entering search terms will make this menu item automatically return the results for the predefined terms."

And last, to delete from the sys.ini all the existing strings of type COM_FINDER_CONFIG_etc

itbra · 2014-04-01T10:18:44Z

This is quite confusing. You said, you'll to integrate the move of all strings that do not belong to my PR from .sys.ini to .ini via a new PR, which is why i only edited my language strings. Those you mention above are not part of my PR.

Now i don't know what to move where. Just to get things clear: You want me to move all langage strings from .sys.ini to .ini and leave within .sys.ini only this block:

`; Joomla! Project
; Copyright (C) 2005 - 2014 Open Source Matters. All rights reserved.
; License GNU General Public License version 2 or later; see LICENSE.txt, see LICENSE.php
; Note : All ini files need to be saved as UTF-8

COM_FINDER="Smart Search"
COM_FINDER_XML_DESCRIPTION="Smart Search"

COM_FINDER_MENU_SEARCH_VIEW_DEFAULT_TEXT="The default search layout."
COM_FINDER_MENU_SEARCH_VIEW_DEFAULT_TITLE="Search"

infograf768 · 2014-04-01T10:23:54Z

Leave only in sys.ini these 4 strings indeed, BUT NOT MOVE ALL sys.ini strings to ini.
ONLY move there:

you new strings
- these older strings
  COM_FINDER_SEARCH_FILTER_SEARCH_LABEL="Search Filter"
  COM_FINDER_SEARCH_FILTER_SEARCH_DESC="Selecting a Search Filter will limit any searches submitted to use the selected filter."
  COM_FINDER_SEARCH_SEARCH_QUERY_LABEL="Search Query"
  COM_FINDER_SEARCH_SEARCH_QUERY_DESC="Entering search terms will make this menu item automatically return the results for the predefined terms."

itbra · 2014-04-01T10:32:57Z

Done.

infograf768 · 2014-04-02T07:18:20Z

Merged after deleting the redundant CONFIG strings in sys.ini. Thanks

…mla#3285)

piotr-cz · 2016-09-28T09:04:13Z

It's also missing the number of results to show, however functionality to set it is not implemented in Smart search component and falls back to global Default List Limit

piotr-cz · 2016-09-28T11:35:25Z

There is one more parameters missing in both Component conffig and View config: show_search_form.

zero-24 · 2016-09-28T20:31:09Z

Please open a new bug report with all needed information. Comments on a closed / merged PR get missed fast. Thanks

itbra added Needs JoomlaCode Tracker Item labels Mar 12, 2014

brianteeman added Has JoomlaCode Tracker Item and removed Needs JoomlaCode Tracker Item labels Mar 12, 2014

itbra added 2 commits March 24, 2014 13:17

Update default.xml

4a39c00

Added *JGLOBAL_USE_GLOBAL* option to *show_suggested_query* and *show_explained_query parameters*

Update config.xml

9f4fd31

Added *show_suggested_query* and *show_explained_query* parameters to make them configurable and overwritable

chrisdavenport reviewed Mar 24, 2014
View reviewed changes

itbra added 3 commits March 25, 2014 13:08

Update config.xml

d2c2260

Changed default values of *show_suggested_query* and *show_explained_query* to 1. Changed option titles from JYES and JNO to JSHOW and JHIDE

Update en-GB.com_finder.sys.ini

2a27565

Changed translations for *COM_FINDER_SHOW_SUGGESTED_QUERY_* and *COM_FINDER_SHOW_EXPLAINED_QUERY_*

Update default.xml

080be61

Changed field types of *show_suggested_query* and *show_explained_query* from radio to list

chrisdavenport reviewed Mar 31, 2014
View reviewed changes

Update en-GB.com_finder.sys.ini

cc4b67f

Removed question marks

itbra added 3 commits April 1, 2014 11:55

Update en-GB.com_finder.sys.ini

fbc1f29

Added substring *CONFIG_* to new translation keys.

Update default.xml

6c23fb4

Added substring *CONFIG_* to new options

Update config.xml

27a869a

Added substring *CONFIG_* to new language keys.

itbra added 2 commits April 1, 2014 12:32

Update en-GB.com_finder.ini

a8e9852

Update en-GB.com_finder.sys.ini

01f67cc

infograf768 closed this in 9a3c20d Apr 2, 2014

infograf768 added a commit that referenced this pull request Apr 2, 2014

Correcting #3285

e9cf2c5

zero-24 mentioned this pull request Apr 18, 2014

Übersetzung der neuen Optionen von com_finder joomlagerman/joomla#19

Merged

Bakual pushed a commit to Bakual/joomla-cms that referenced this pull request May 12, 2014

[fix][joomla#33441] SmartSearch menu item missing parameters (Fix joo…

07caffb

…mla#3285)

Bakual pushed a commit to Bakual/joomla-cms that referenced this pull request May 12, 2014

Correcting joomla#3285

d981511

piotr-cz mentioned this pull request Sep 28, 2016

Smart search menu item doesn't allow to set some options to Use Global #12186

Merged

Uh oh!

Conversation

itbra commented Mar 12, 2014

Uh oh!

chrisdavenport commented Mar 23, 2014

Uh oh!

itbra commented Mar 24, 2014

Uh oh!

chrisdavenport commented Mar 24, 2014

Uh oh!

itbra commented Mar 24, 2014

Uh oh!

Bakual commented Mar 24, 2014

Uh oh!

itbra commented Mar 24, 2014

Uh oh!

itbra commented Mar 24, 2014

Uh oh!

chrisdavenport Mar 24, 2014

Choose a reason for hiding this comment

Uh oh!

itbra Mar 25, 2014

Choose a reason for hiding this comment

Uh oh!

chrisdavenport commented Mar 24, 2014

Uh oh!

chrisdavenport Mar 24, 2014

Choose a reason for hiding this comment

Uh oh!

itbra Mar 25, 2014

Choose a reason for hiding this comment

Uh oh!

chrisdavenport Mar 31, 2014

Choose a reason for hiding this comment

Uh oh!

itbra Mar 31, 2014

Choose a reason for hiding this comment

Uh oh!

chrisdavenport commented Mar 31, 2014

Uh oh!

infograf768 commented Mar 31, 2014

Uh oh!

itbra commented Mar 31, 2014

Uh oh!

infograf768 commented Mar 31, 2014

Uh oh!

chrisdavenport commented Mar 31, 2014

Uh oh!

itbra commented Apr 1, 2014

Uh oh!

infograf768 commented Apr 1, 2014

Uh oh!

itbra commented Apr 1, 2014

Uh oh!

infograf768 commented Apr 1, 2014

Uh oh!

itbra commented Apr 1, 2014

Uh oh!

infograf768 commented Apr 2, 2014

Uh oh!

piotr-cz commented Sep 28, 2016

Uh oh!

piotr-cz commented Sep 28, 2016

Uh oh!

zero-24 commented Sep 28, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants