Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possibility to remove filters individually when using the searchtools layout helper #8767

Closed
wants to merge 3 commits into from

Conversation

panopt
Copy link
Contributor

@panopt panopt commented Dec 22, 2015

Bring up a 'removedFilters' (array) option to the searchtools layout helper that allows to remove filters individually.

Testing

To test it you can change the file:
administrator/components/com_content/views/articles/tmpl/default.php

on line 48:

echo JLayoutHelper::render('joomla.searchtools.default', array('view' => $this));

by:

echo JLayoutHelper::render('joomla.searchtools.default', array(
        'view' => $this,
        'options' => array(
                'removedFilters' => array()
        )
)); 

Then, add names of the searchtools fields you want to remove to the 'removedFilters' option array :

        'options' => array(
                'removedFilters' => array('filter_published', 'filter_category_id')
        )

and observe the result (filters removed) on the searchtools of:
/administrator/index.php?option=com_content&view=articles


capture du 2015-12-22 16 39 14ok
(screenshot: searchtools with all filters)


capture du 2015-12-22 16 34 10
(screenshot: searchtools with 'status' and 'category' filters removed)

… layout helper

Bring up a 'removedFilters' (array) option to the 'searchtools layout helper' that allows to remove/disable filters individually.

# Testing

To test it you can change the file:
administrator/components/com_content/views/articles/tmpl/default.php
on line 48:
echo JLayoutHelper::render('joomla.searchtools.default', array('view' => $this));
by: 
echo JLayoutHelper::render('joomla.searchtools.default', array(
        'view' => $this,
        'options' => array(
                'removedFilters' => array()
	)
)); 

Then, add one or more searchtools field name to the 'removedFilters' option array :
        'options' => array(
                'removedFilters' => array('filter_published', 'filter_category_id')
	)

and observe the result (filters removed) on the searchtools bar of:
/administrator/index.php?option=com_content&view=articles
@panopt panopt changed the title Remove filters individually when using the searchtools layout Possibility to remove filters individually when using the searchtools layout helper Dec 22, 2015
if (isset($data['options']['removedFilters']))
{
foreach ($data['options']['removedFilters'] as $removedFilters){
if (isset($filters[$removedFilters]))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to check that a variable is set before you unset it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, please in the foreach drop the { to next line to make Travis happy.

I have tested this successfully when testing #8633

@panopt
Copy link
Contributor Author

panopt commented Dec 23, 2015

Thank you,
This is updated.

@infograf768
Copy link
Member

Works for me when testing #8633

@AnishaTailored
Copy link

I have tested this item ✅ successfully on 307aada


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8767.

1 similar comment
@gunjanpatel
Copy link
Contributor

I have tested this item ✅ successfully on 307aada


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8767.

@gunjanpatel
Copy link
Contributor

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8767.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 14, 2016
@roland-d
Copy link
Contributor

@panopt What is the use case here? Each view uses it's own filter_xx.xml file If you do not want certain fields you can leave them out.

Can you fix the conflicts too? Thank you.

@ggppdk
Copy link
Contributor

ggppdk commented Jul 16, 2016

@roland-d

i think a usage can be to display 1 or 2 important filters outside the sliders ?
i do it in some cases,

  • and current code does not allow such usage, and that is 1 of the reasons, that i don't use it

@gunjanpatel
Copy link
Contributor

@panopt Please sync this branch. As it has conflicts. Thanks

@roland-d
Copy link
Contributor

@ggppdk Thank you for your input. If you want to display 1 or more filters outside the slider wouldn't you do a layout override?

From what I understand you would use 1 separate layout for placing the 1 or more filters outside the slider and another layout for creating the slider? That would be the same as creating your own layout as an override for the search tool layout.

@wilsonge What do you think of this change?

@ggppdk
Copy link
Contributor

ggppdk commented Jul 19, 2016

@roland-d
layout override ?
i meant custom layout for custom component

another reason for not using it is that i consider it sometimes a little confusing:
confusing

i think a hover tooltip like "author", "access", will be good since label is hidden,
clicking once to open it and once more to close it to see the type of filter is annoying,

but ok you get used to the place that every filter is ...

@panopt
Copy link
Contributor Author

panopt commented Jul 19, 2016

I found a case where filters are hard coded in the template:
administrator/components/com_categories/views/categories/tmpl/modal.php (see #8633)

So I thought "Damn, redundant code in several places ! This must be painful to maintain."

Note : It's even possible to go further since 'searchtools layout helper' brings standard filters that corresponds to the meta functionalities that should be implemented in many components : status, language, acces, author, categories, tags.

@panopt panopt closed this Jul 19, 2016
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 19, 2016
@panopt panopt reopened this Jul 19, 2016
@roland-d
Copy link
Contributor

@ggppdk Sorry for my thickness but I still don't see the issue. Let me try to explain myself again :)

If you have a search tool filter as shown in the main description of this thread. That is controlled by the XML you pass to it. So if you don't want some filters to show, then don't add the options in the XML. That is what I am thinking.

As for your other point that the filters can be unclear and can be clarified with a tooltip. Yes I can see that working.

@wilsonge
Copy link
Contributor

I guess the point is that you can't template override a xml file but you can template override a view?

@brianteeman
Copy link
Contributor

Not being able to override an xml is potentially an issue moving forward. I
hit it yesterday trying to do some admin template redesigns

On 26 July 2016 at 09:50, George Wilson notifications@github.com wrote:

I guess the point is that you can't template override a xml file but you
can template override a view?


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#8767 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8W982xrGI5Rv4-_SvN8wlmgPtN3Jks5qZco9gaJpZM4G6Bgc
.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

@wilsonge
Copy link
Contributor

wilsonge commented Jul 26, 2016

XML files shouldn't be overridable in full. If you wish to skip fields you should render fields individually (rather than a full fieldset at once which we kinda do to cut corners) - or you edit the xml file on load in a plugin event.

Having said that adding shortcuts for 'easy' stuff like this is of course feasible

@dgrammatiko
Copy link
Contributor

@phproberto I think you have a nice solution on this XML override 😉

@ggppdk
Copy link
Contributor

ggppdk commented Jul 26, 2016

@roland-d

filtering implementation is good but, not enough for all cases

In some listings

  • i would need 1 or 2 important filters to be out of the slider
  • so i would need to call it twice ...

that is 1 of the reasons, that i prefer to use custom filtering solution in my views

@panopt
Copy link
Contributor Author

panopt commented Jul 26, 2016

I am embarrassed to let you down when I see all these reactions (thank you), but I do not have time anymore to take care of this snippet (I submitted it 6 months ago...).
Anyway a smarter solution has been proposed for the pointed problem ?
Can I close this patch ?

@mbabker
Copy link
Contributor

mbabker commented Jul 26, 2016

I think you have a nice solution on this XML override

I don't agree with the notion of creating overrides for XML forms. Yes, this isn't the "user friendly" way to do it, but that's exactly why there's the onContentPrepare(Data|Form) events in the MVC layer. Sadly, JForm itself has no hooks so you're stuck relying on something else to give you integration points for form manipulation.

Truth be told, I don't think we want to make form overrides as easy as template overrides because this gets into a point where you're manipulating data (structures and values) and you really need to understand what you're doing when you get here.

I know the context of what we're talking about is search tools, but ultimately that XML file is loaded into a JForm instance. So unless you're going to add more inconsistent behaviors at the MVC layer, whatever you do for this use case involving XML overrides affects all uses of JForm.

@wilsonge
Copy link
Contributor

@panopt you definitely haven't let us down. Your proposal has sparked conversation which is even more important :) Thank you for everything you have done!

@ggppdk
Copy link
Contributor

ggppdk commented Jul 26, 2016

@panopt

anymore to take care of this snippet (I submitted it 6 months ago...).
Anyway a smarter solution has been proposed for the pointed problem

It should feel normal that some PRs, stay open for quite long, no frustration needed,

(i have felt it for a PR, that took considerable time, and now i will only do small PRs, unless i feel that there is good chance to be accepted)

Changes we propose effect others and can not be removed easily later, if they are found to be bogus or bad design choice, because they will be a B/C break

and I think you do not have to wait for anything

If you have not done the already:
you can copy: /layouts/joomla/searchtools/
to custom folder

  • customize it according to your needs ...., and load it ...

Just change ...

// Search tools bar
echo JLayoutHelper::render('joomla.searchtools.default', array('view' => $this));

Then when/if this change is accepted you,
you can use default layout later,

or you can just close this

@ggppdk
Copy link
Contributor

ggppdk commented Jul 26, 2016

Also if the purpose is just to hide it, add some targeted CSS , or to move some filter add some JS code

@roland-d
Copy link
Contributor

@wilsonge Is this something to take into J4?

@ghost
Copy link

ghost commented May 27, 2017

gently Reminder @wilsonge


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/8767.

@ghost
Copy link

ghost commented Jun 22, 2017

Reminder @wilsonge


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/8767.

@wilsonge
Copy link
Contributor

We need to ensure that all our search tools are in xml forms (with appropriate plugin triggers for manipulation). But in my opinion I don't see the need for this in core

@ghost
Copy link

ghost commented Jun 22, 2017

thanks @wilsonge.

If no other Decision i will close this PR at 23. July 2017.

@infograf768
Copy link
Member

FYI, we did manipulate search tools to display exactly what we wanted in function of the situation, see example of code here:
https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_associations/views/associations/view.html.php#L132-L136

@joomla-cms-bot
Copy link

Set to "closed" on behalf of @franz-wohlkoenig by The JTracker Application at issues.joomla.org/joomla-cms/8767

@ghost
Copy link

ghost commented Jul 23, 2017

closed as stated above.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/8767.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet