Omeka
Omeka copied to clipboard
omeka
Removable item search filters
Adding possibility to display item search filters as links, and on click removing the filter. It is extending PR #702
BTW: I saw issue #705 which may change the way how search filters are generated, so I understand if it's not time for this change yet.
It might be better to always pass the "id" information along, and then just have the "show remove links" flag be processed in the view partial. You'd still need the logic in there for dealing with a missing ID to account for plugins, but you'd be able to remove a little complexity from the view helper side, and you wouldn't need to pass the "$options" into the filter (plugins can just update to provide the ID in what they return and don't have to worry about also checking if remove_filter is on or not to decide whether they should provide it).
Are you planning to switch to getting just the query string params as your TODO notes?
It's probably preferable to just implement the remove filter flag as a direct boolean flag in the signature of the helper with a false default value.
I'm sorry, I missed somehow your comment. I don't really understand the idea with 'id' information passed along. Could you show code snippet or rewrite some line of code, so I can better understand? The solution I proposed is meant to work by default as it has been working before, so no one needs to rewrite their plugins. Without passing '$options' into filter, I don't see any way of making it possible, did I miss something?
Yes, I would like to get just the query string as I note in TODO, I just wanted to verify first if it's OK. Do you see any possible problem with this change?
Then I don't understand exactly what you mean with remove filter flag in the signature of helper? Instead of default value in $options, that are merged, would you prefer to compare it like this?
if (!empty($options['remove_filter'])) { $this->removeFilter = true; }
I think I understood your first point about always passing the 'id' information so I did that and simplified a bit the helper and view. Then I uncommented the TODO part with getting only the actual query params. I still don't know about the last point and the remove flag in signature of helper, but I guess it's easier now to show me what you mean.
I think what I was talking about with the flag and signature was to not pass an options array, but just have a single direct boolean argument to the method for whether to show those remove links, since that's the only option there is.
Ok, get it, thanks. I was thinking about another possible $option, which could be class for the link, currently 'search-filter'. It's in own view, so it's easier to alter it in public themes, so maybe not needed. I just thought there might be need for more $options in future and then it's good if we can just add more keys, instead of whole new argument. But I'm OK with making it as bool argument, if you don't see need for any other options.
You didn't say it sounds like a good idea, so I made the removable filter flag into an argument to the helper method.
After using it for some while, I often needed additional parameters/options in custom theme view, so I added again the $options array. Maybe bad design to add argument with unclear structure, but I find it really helpful when theming the filters.