Issues in Smart Search "Advanced Search"

[smz]

The issue:

When the "Expand Advanced Search" option is selected, the Advanced Search form should be displayed as expanded: this does not happens.

The fix:

Add the in class to the advancedSearch <div> when the option is selected

More issues NOT SOLVED by this PR

• The JS code for toggling the Advanaced Search form is not functional and can be eliminated (the functionality is guaranteed by BS data-toggle)
• The JS that should disable fields not used in the search when submitting the form (and thus clean-up the search URL) is not working and search URLs are littered by unused options (e.g.: &Search=&t[]=&t[]=&t[]=)

P.S.: The above issues have been solved too

[smz]

The first outstanding issue is trivial: just delete the associated code.

The second one is more tricky and out of my reach: I think it is more @dgt41 territory...

As the second outstanding issue is definitely more important and more difficult to solve, anyone taking it into his hands should feel free to open a new PR and incorporate my fix for the lesser issue into it.

I didn't touch the associated JS (not even by deleting the useless code) so that we can start with a clean situation.

[smz]

EASY! Wrong target div name!! Commit is on its way... 😄

[smz]

With the latest commit this PR solves also the "outstanding" issues.
Ready to test.

[smz]

... there is something I still don't like: all select boxes have name="t[]"

I don't think this is correct, but it is something that must probably be searched in JHtml::_('filter.select')

And also the &search= at the end of the URL if none of the select boxes are used should be eliminated...

[smz]

probably not: all the t[], if I get it right, are for building a taxonomy array and as far as regard the empty &search=... WTF!

Go for testing!! 👍

[dgrammatiko]

@smz Right now I am on the move and cannot spare some time for this and next week is kinda holidays… Anyway just seeing what you’ve done here I have to warn you that all this code you removed is probably the same from bootstrap slider and it shouldn’t be removed but fixed. The idea behind this, its that this affects front end and some templates may not use BS so in that case the front end won’t be functional.
To cut it sort: don’t depend on BS code (esp for front end) as that my not be available!

[smz]

@dgt41 Dimitris, the code I removed wasn't working since the times when someone decided that advancedSearch is a nicer ID than advanced-search. That happened in 2012 (August 16), when Joomla was "bootstrapped". I seriously doubt we are going to break B/C by removing that broken code, but admitting there is a 0.0001% probability of breaking a site, I firmly think that B/C cannot be guaranteed when it is based on existing bugs.

Anyone not using BS in front-end has the responsibility to set-up his working framework and the needed overrides with associated JS. We cannot throw random (and not working) JS in our views assuming that this may be useful in different scenarios based on who-knows-what framework and views.

[smz]

@dgt41 ... and "fixing" that code by adapting it to the correct target ID will in any case break that 0.0001% of (totally hypothetical) sites who are still using a non-BS framework.

But even more, if they are still using the old advanced-search ID in their views (condition needed for the current JS is working for them), that means they are using an overridden view and thus this mod will not "reach" them.

Conclusion: 100% B/C

[smz]

Found a new issue and fixed that too: in the "Result Description" dates conditions ("before", "after" and "exact") were not translated.

[smz]

Sorry if this became an "omnibus" PR, but it is a case of serendipity in finding new issues starting from a single tiny one...

To recap, for testers, this PR now fixes the following:

• The "Expand Advanced Search" option is now honored
• Some broken and useless JS for toggling the (old) "Advanced Search" slider has been removed
• In absence of advanced search options the returned URL is fixed from &Search=&t[]=&t[]=&t[]= to just &Search= (TODO: remove also that remnant? A router job, I guess...)
• The "before", "after" and "exact" keywords used to qualify search dates in the "Search description" are now translatable using the following strings:

COM_FINDER_QUERY_DATE_CONDITION_AFTER="after"
COM_FINDER_QUERY_DATE_CONDITION_BEFORE="before"
COM_FINDER_QUERY_DATE_CONDITION_EXACT="exactly on"

[dgrammatiko]

@smz I see it as a wiser choice to leave the script as is and just add the missing -. Please DON’T introduce Bootstrap dependency here. Though you can make an override for protostar with bootstrap code.

[smz]

@dgt41 I'm afraid you're missing the point: the bloody thing is (and was) already BS. I didn't change anything to make it more "BS dependent". Maybe you don't have the code at hand, but, when you can, give it look and also try to comment out the slider JS: "wow! slider still working... WTF!" (that was my reaction when I did it while trying to fix the "Expand Advanced Search" option into that piece of code...). I wasted hours into the damn JS trying to figure out HOW it could work, and at the end... it simply wasn't! Since 2012.

What I would like to do with this PR is to have things working, now, for 3.4.0. I'm not breaking anything and I'm fixing 3 bugs.

If you think that in the future we should be less BS dependent, I can agree with you and when we will move all of this to layouts we can make wonders for that, but IMHO this is not the timing.

[dgrammatiko]

@smz if you just add the - without touching the script, the problem is gone? If yes then just do that, and everybody will be happy!

[smz]

No, Dimitris, because the JS is also handling the cleanup for the unused advanced fields (and hence the URL).

To fix it I should modify the script by changing the target and at that point we will have two colliding JS functions (the BS and "ours") handling the slider.

If I instead just fix the target only in the "fields cleanup" part of it and leave the slider JS as it is we will have a JS targeting a non-existent slider. What will be the sense in that?

[dgrammatiko]

OK I just stated that this is the wrong way (my opinion). You introduce BS dependency and therefore this cannot be undone before v4.0
Again for me this shouldn't be fixed this way, we should do it properly: css framework free

[smz]

@dgt41 BTW, I just tried to follow your suggestion and just added the - to the advanced search <div>: as expected nothing works anymore.

That JS is simply a left-over of the times we had a totally different HTML structure and must be adjusted to the current one.

As far as regards the "slider toggling" part of it adjusting is removing.

[smz]

Dimitris: I A M N O T I N T R O D U C I N G B S D E P E N D E N C Y
it is still was already there! 😄

[smz]

Historically, BS dependency, here, has been introduced on 2012 August 16, by @kyleledbetter with his commit cd663c0 titled (guess what?) "Bootstrap the site."

[stellainformatica]

I've tested the patch with the staging, but I see only the suggestions, not the advanced search fields (see attachment)

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5463.}

[smz]

That's strange... I have it in my test site (current staging):

Just to be sure: do you have articles in your site, have published all the "Smart Search" plugins and performed an "Index" in "Smart Search" backend?

[chrisdavenport]

@stellainformatica Do you have advanced search enabled in the options? I think it's disabled by default.

[stellainformatica]

Sorry, I forgot to make the Index before testing :P
After making the Index the search fields correctly appear.
I have tested with the default template Protostar with Bootstrap

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5463.}

[smz]

👍

[smz]

@chrisdavenport We still have this open for com_finder...

[anibalsanchez]

@test OK

After patch, it works fine

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5463.}

[zero-24]

Thanks for testing, commenting and coding -> RTC

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5463.}