Smart Search - move Filters - v2 to correct merge conflict #8140

pe7er · 2015-10-24T13:47:22Z

New PR to solve Merge Conflict of PR #6959 "Smart Search - move Filters (3x) from left to [Search Tools] "

Please follow the testing procedure of PR #6959

This PR moves the Filter option of all three views of Smart Search to the [Search Tools] button.
(See Issue #6941 regarding the inconsistency with Filter options).

Test instructions

You have to enable the Content - Smart Search Plugin, and Index your test website
Please check the 3 different views before & after the patch:
test all the filters and search box, clear results, choose max amount of results on a page.

Smart Search: Manage Indexed Content

Check the current position of the Filters

In back-end > Components > Smart Search > Indexed Content
The Filters are on the left hand side

This PR changes moves the Filter

from the left to the middle column and will be visible when you click on the [Search Tools] button.

Smart Search: Manage Content Maps

Check the current position of the Filters

In back-end > Components > Smart Search > Content Maps
The Filters are on the left hand side

This PR changes moves the Filter

from the left to the middle column and will be visible when you click on the [Search Tools] button.

Smart Search: Manage Search Filters

Check the current position of the Filters

In back-end > Components > Smart Search > Search Filters
The Filters are on the left hand side

This PR changes moves the Filter

from the left to the middle column and will be visible when you click on the [Search Tools] button.

Note

To Clear the Filters / Search box, you can use the [Clear] button.
However, sometimes you have to press 2 times to clear the results.
I don't know the cause and how to solve it...

jduerscheid · 2015-10-24T15:07:41Z

I have tested this item ✅ successfully on 9a763fb

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

roland-d · 2015-10-24T15:34:24Z

administrator/components/com_finder/views/index/tmpl/default.php

-				<?php echo $this->pagination->getLimitBox(); ?>
-			</div>
-		</div>
+		<?php echo JLayoutHelper::render('joomla.searchtools.default', array('view' => $this)); ?>
 		<div class="clearfix"> </div>


@pe7er I think the clearfix should be removed here. You did that in the old PR. Can you check please?

pe7er · 2015-10-24T18:02:52Z

@roland-d I think that JHtmlSidebar::setAction can be removed.
I've removed the clearfix in all three list views.

n9iels · 2015-10-31T21:49:51Z

I applied the patch before activating the content plugin. The message that tells me that is now very close to the search filters. Maybe we can add some padding there?

I also noticed that the filter on "Content maps" are always visible, and for some reason the filter on the right is always marked as an active filter (There are blue lines around it).
On other places where we use this filters (com_content for example) the blue line only appears when we choose an option of the filter.

…earch-tools-filters-smartsearch2

joomla-cms-bot · 2015-11-03T21:46:37Z

This PR has received new commits.

CC: @jduerscheid

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

pe7er · 2015-11-03T21:49:15Z

Thanks @n9iels, I corrected the Branch filter on "Content maps".
Now it is not activated automatically anymore.

btw: I did not change the padding of the warning message...

n9iels · 2015-11-04T09:07:42Z

Yes, that works for me! 😄 I see now the same problem appears for "indexed content", the search filter "Select type of content" is always active over there.

Also the search filter on "Content mapping" are always visible on that page. Or is that done by a certain reason?

n9iels · 2015-11-04T09:18:38Z

About that padding and the message. Strange enough the message appears below the search filters. Other messages like "save" and "delete" appear above the messages.

Seems like all other message than "successfully saved", "successfully deleted" etc. appear below the filters. Not a problem for this PR I think so.
I will make a PR for that 👍 (after this PR and others are merged, otherwise that merge conflicts never solves 😉 )

pe7er · 2015-11-04T12:06:34Z

Thanks for testing!

No, I did not set the filter to "always active" on purpose. I'll check if I can correct it.

If someone knows the cause of the problem, please correct it with a PR,
or tell me what to change & I'll do so...

roland-d · 2015-11-04T14:38:16Z

@pe7er The "always active" happens because you have set a default value of 0 here: https://github.com/joomla/joomla-cms/pull/8140/files#diff-762a4c09ff9b1a26880f1391232575c2R23

The first option you give to the dropdown has a value of 0:
https://github.com/joomla/joomla-cms/pull/8140/files#diff-762a4c09ff9b1a26880f1391232575c2R29

that is why the first entry becomes an active one. I guess you can drop the default value setting.

…earch-tools-filters-smartsearch2

joomla-cms-bot · 2015-11-04T15:41:11Z

This PR has received new commits.

CC: @jduerscheid

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

joomla-cms-bot · 2015-11-04T16:21:52Z

This PR has received new commits.

CC: @jduerscheid

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

zero-24 · 2015-11-04T16:35:39Z

@pe7er Travis cry

FILE: ...d/joomla/joomla-cms/administrator/components/com_finder/models/maps.php
--------------------------------------------------------------------------------
FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
 176 | ERROR | Expected "if (...)\n...{...}\n...else\n"; found "if
     |       | (...)\n...{...}\n...else"
 184 | ERROR | Expected "if (...)\n...{...}\n...else\n"; found "if
     |       | (...)\n...{...}...else"
--------------------------------------------------------------------------------
UPGRADE TO PHP_CODESNIFFER 2.0 TO FIX ERRORS AUTOMATICALLY
--------------------------------------------------------------------------------

roland-d · 2015-11-04T16:52:57Z

I don't think the last change is correct. In Joomla 3.4.5 the dropdown looks like this:

Branches only as first option and I think we should keep that.

joomla-cms-bot · 2015-11-04T18:46:48Z

This PR has received new commits.

CC: @jduerscheid

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

pe7er · 2015-11-04T18:48:03Z

I removed my previous PR "fixed Select Branch dropdown in Smart Search Content Maps"

dgrammatiko · 2015-11-04T21:37:51Z

Looks good to me:

roland-d · 2015-11-04T21:39:19Z

@dgt41 Compare it to the implementation in J3.4.5, it is quite different. The current implementation uses it's own JHtml class, that is completely skipped now.

dgrammatiko · 2015-11-04T21:40:15Z

ahh yeah, the finder way 😝

…earch-tools-filters-smartsearch2

joomla-cms-bot · 2015-11-04T22:16:22Z

This PR has received new commits.

CC: @dgt41, @jduerscheid, @Kubik-Rubik

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

pe7er · 2015-11-04T22:18:30Z

I've removed the hardcoded SQL
query="SELECT id AS value, title AS branch FROM #__finder_taxonomy WHERE parent_id = 1"
from /administrator/components/com_finder/models/forms/filter_maps.xml and
added /administrator/components/com_finder/models/fields/branches.php
to retrieve the branches.

roland-d · 2015-11-04T22:18:41Z

administrator/components/com_finder/models/maps.php

@@ -263,7 +264,7 @@ protected function populateState($ordering = null, $direction = null)
 		$state = $this->getUserStateFromRequest($this->context . '.filter.state', 'filter_state', '', 'string');
 		$this->setState('filter.state', $state);

-		$branch = $this->getUserStateFromRequest($this->context . '.filter.branch', 'filter_branch', '1', 'string');
+		$branch = $this->getUserStateFromRequest($this->context . '.filter.branch', 'filter_branch', '', 'string');


Don't we have to put back the default value here?

dgrammatiko · 2015-11-04T22:40:03Z

@pe7er I am afraid that administrator/components/com_finder/models/fields/branches.php doesn;t cover all the functionality of the old JHtml class https://github.com/joomla/joomla-cms/blob/staging/components/com_finder/helpers/html/filter.php#L208-L394

roland-d · 2015-11-04T22:53:15Z

administrator/components/com_finder/models/fields/branches.php

+	public function getOptions()
+	{
+		// Build the query.
+		$db = JFactory::getDbo();


@pe7er Just change this complete function to contain:

return JHtml::_('finder.mapslist');

joomla-cms-bot · 2015-11-04T23:02:23Z

This PR has received new commits.

CC: @dgt41, @jduerscheid, @Kubik-Rubik

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

pe7er · 2015-11-04T23:03:31Z

Removed the SQL query from fields/branches.php & replaced with return JHtml::_('finder.mapslist');
Thanks for the tip @roland-d !

dgrammatiko · 2015-11-04T23:11:51Z

@pe7er can you also implement this #8140 (comment)
so I can re-test this?

File administrator/components/com_finder/views/index/tmpl/default.php

pe7er · 2015-11-04T23:14:39Z

@dgt41 yes, please!
I think that we should correct the layout issue with the message later because it might need to be corrected at other places as well...

dgrammatiko · 2015-11-04T23:22:58Z

Works fine here, one small UX tho:

coolcat-creations · 2015-11-04T23:23:35Z

Small issue:

Before:

After patch:

coolcat-creations · 2015-11-04T23:26:25Z

I don´t know if this regards this PR but if i don´t have published filters there is a message that "No filters have been created yet. Create a filter." - that´s not true because there are filters but they are unpublished.

pe7er · 2015-11-04T23:27:38Z

Thanks for testing @dgt41 & @designbengel !
Those UX issues need to be corrected, but I think that it could be done for beta2 in another PR.

coolcat-creations · 2015-11-04T23:28:35Z

There is a Message that no content has been indexed by filtering unpublished items?

roland-d · 2015-11-04T23:30:56Z

@designbengel I think this is an existing issue, this PR only moves the buttons from the sidebur to the Search Tools bar. Perhaps raise a separate issue for that?

coolcat-creations · 2015-11-04T23:31:18Z

I have tested this item ✅ successfully on b14cbd4

Ok, so like @roland-d says it´s regarding just the location-change of filters - so test for this is ok :)

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

infograf768 · 2015-11-27T16:45:44Z

Folks, I can't see
COM_FINDER_MAPS_SELECT_BRANCHE="- Select Branch -"
anywhere.

infograf768 · 2015-11-28T06:27:55Z

@pe7er @roland-d

I have the feeling some new lang strings were merged here that should'nt have...

infograf768 · 2015-11-28T07:25:09Z

@pe7er @roland-d Please test
#8558

Correcting smartsearch filter #8140

Smart Search - move Filters - v2 to correct merge conflict

9a763fb

joomla-cms-bot added Language Change This is for Translators PR-staging labels Oct 24, 2015

pe7er mentioned this pull request Oct 24, 2015

Smart Search - move Filters (3x) from left to [Search Tools] #6959

Closed

roland-d reviewed Oct 24, 2015
View reviewed changes

removed clearfix

2fe05b9

zero-24 added this to the Joomla! 3.5.0 milestone Oct 24, 2015

pe7er added 2 commits November 3, 2015 22:43

Merge branch 'staging' of https://github.com/joomla/joomla-cms into s…

64fae2e

…earch-tools-filters-smartsearch2

corrected Select Branch filter

df5f6f2

pe7er added 2 commits November 4, 2015 16:30

Merge branch 'staging' of https://github.com/joomla/joomla-cms into s…

ef2c89b

…earch-tools-filters-smartsearch2

changed default option value of filter_index.xml

0f93069

fixed Select Branch dropdown in Smart Search Content Maps

fa198ab

undoing: fixed Select Branch dropdown in Smart Search Content Maps

26670dd

pe7er added 2 commits November 4, 2015 22:48

Merge branch 'staging' of https://github.com/joomla/joomla-cms into s…

1a321f0

…earch-tools-filters-smartsearch2

branches now retrieved via fields

069aa85

roland-d reviewed Nov 4, 2015
View reviewed changes

removed SQL query from fields/branches.php

b14cbd4

roland-d closed this in 6766133 Nov 4, 2015

pe7er deleted the search-tools-filters-smartsearch2 branch November 5, 2015 00:08

infograf768 added a commit to infograf768/joomla-cms that referenced this pull request Nov 28, 2015

Correcting smartsearch filter joomla#8140

dbc02cb

infograf768 mentioned this pull request Nov 28, 2015

Correcting smartsearch filter #8140 #8558

Merged

rdeutz added a commit that referenced this pull request Nov 29, 2015

Merge pull request #8558 from infograf768/finder_filter

6b57657

Correcting smartsearch filter #8140

Smart Search - move Filters - v2 to correct merge conflict #8140

Smart Search - move Filters - v2 to correct merge conflict #8140

Conversation

pe7er commented Oct 24, 2015

Test instructions

Smart Search: Manage Indexed Content

Check the current position of the Filters

This PR changes moves the Filter

Smart Search: Manage Content Maps

Check the current position of the Filters

This PR changes moves the Filter

Smart Search: Manage Search Filters

Check the current position of the Filters

This PR changes moves the Filter

Note

jduerscheid commented Oct 24, 2015

roland-d Oct 24, 2015

Choose a reason for hiding this comment

pe7er commented Oct 24, 2015

n9iels commented Oct 31, 2015

joomla-cms-bot commented Nov 3, 2015

pe7er commented Nov 3, 2015

n9iels commented Nov 4, 2015

n9iels commented Nov 4, 2015

pe7er commented Nov 4, 2015

roland-d commented Nov 4, 2015

joomla-cms-bot commented Nov 4, 2015

joomla-cms-bot commented Nov 4, 2015

zero-24 commented Nov 4, 2015

roland-d commented Nov 4, 2015

joomla-cms-bot commented Nov 4, 2015

pe7er commented Nov 4, 2015

dgrammatiko commented Nov 4, 2015

roland-d commented Nov 4, 2015

dgrammatiko commented Nov 4, 2015

joomla-cms-bot commented Nov 4, 2015

pe7er commented Nov 4, 2015

roland-d Nov 4, 2015

Choose a reason for hiding this comment

dgrammatiko commented Nov 4, 2015

roland-d Nov 4, 2015

Choose a reason for hiding this comment

joomla-cms-bot commented Nov 4, 2015

pe7er commented Nov 4, 2015

dgrammatiko commented Nov 4, 2015

pe7er commented Nov 4, 2015

dgrammatiko commented Nov 4, 2015

coolcat-creations commented Nov 4, 2015

coolcat-creations commented Nov 4, 2015

pe7er commented Nov 4, 2015

coolcat-creations commented Nov 4, 2015

roland-d commented Nov 4, 2015

coolcat-creations commented Nov 4, 2015

infograf768 commented Nov 27, 2015

infograf768 commented Nov 28, 2015

infograf768 commented Nov 28, 2015