[#35967] - [4.1] finder show_search_form
- Fixed in Code Base
- 14 Mar 2022
- Medium
- Build: 4.0-dev
- # 35967
- Diff
- brianteeman:finder
User tests: Successful: Unsuccessful:
This if statement (which defaults to true) is pointless as the actual param it is checking does not exist so it can only ever be true
Code Review only to confirm that the param doesn't exist
Pull Request for Issue #35692
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Front End com_finder |
Brian i don't find the approach very neat.
For example in my case i use that param to hide the search form in my results, since i am only using the search module. A param can be useful independently to it's GUI representation.
The param is never defined anywhere!
I think it is much better to define it to the menu item and/or the com_finder's parameters.
It is really useful in case both the search module and the search form are being loaded. In that case you will get double search forms.
How can it be used today?
How can it be used today?
Not sure that i understand the question.
Please try the following:
Visit a site that uses a search mod at the top (as usual) and search for something.
You will now get 2 search forms (1 from the module and 1 from the results).
I can try to create a PR implementing that param.
In my last attempt i was getting a js error when the search form was not loaded and may need some help there.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35967.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35967.
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35967.
Please do not to merge that.
It is really useful, especially if the search module is used as well.
I am planning to create a PR that allows to hide the search form utilizing that param.
@brianteeman Is it okay with you to remove the RTC and let @sakiss do a PR with the params implementation?
its been 7 weeks since they said they were going to create a new pr after failing.
that would have to be for 4.1 as it would be a new feature.
this is fixing a bug in 4.0 and it doesnt prevent the new feature being added at a later date if that ever arrives.
Please dont forget merging this
| Labels |
Added:
?
?
|
||
Rebased to 4.1
| Title |
|
||||||
@brianteeman could you please check #37254 ? Thanks
I have checked it and as I expected it is useless. Probably why the promised pr was not delivered in 3 months
| Labels |
Added:
?
Removed: ? |
||
@brianteeman where did you see a promise for PR?
Please do not to merge that. It is really useful, especially if the search module is used as well. I am planning to create a PR that allows to hide the search form utilizing that param.
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-03-14 04:36:53 |
| Closed_By | ⇒ | bembelimen |
Fair enough, thx
Thanks
@brianteeman Well this is far from a promise.
The only reason i did not go on, was because i needed some assertion that this PR was not going to get merged and i will not waste my time for nothing.
Anyway if @bembelimen thinks that this should be fixed through the settings, my offer is still valid, but i may need some help in some parts.
Feel free to make a PR, if you cover all cases I'm happy.
I have tested this item✅ successfully on 015b9ce
Code Review
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35967.