The facet and unclick link code is a bit ugly - I think we can separate things a bit to make the code easier to understand
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | facet_cleanup-463900-5.patch | 12.69 KB | pwolanin |
| #4 | facet_cleanup-463900-4.patch | 11.05 KB | JacobSingh |
| #3 | facet-cleanup-463900-3.patch | 9.85 KB | pwolanin |
| #1 | facet-cleanup-463900-1.patch | 9.85 KB | pwolanin |
Comments
Comment #1
pwolanin commentedComment #2
David Lesieur commentedPatch seems to work well.
By moving the $count argument of theme_apachesolr_facet_item() after $path and $options, it would be more consistent with theme_apachesolr_unclick_link().
Comment #3
pwolanin commentedrevised patch per suggestion.
Comment #4
JacobSingh commentedWhile reviewing the patch, I uncovered this bug: http://drupal.org/node/495012
Which is not related to the patch itself.
The patch itself looks okay, I don't think it will be totally intuitive for most themer. For instance ,we have apachesolr_facet_item and apachesolr_unclick_link. Should it be apachesolr_add_facet_link and apachesolr_remove_facet_link?
Anyway, here is another one with a little bit of docs in apachesolr.module as a starting point.
Comment #5
pwolanin commentedfor consistency, changing apachesolr_facet_item to apachesolr_facet_link, but I think longer names are unwieldy.
Also, making the signature of apachesolr_sort_link match.
Comment #6
JacobSingh commentedI didn't test this last patch, but if that's all you changed I give it a thumbs up.
Comment #7
pwolanin commentedcommitted to 6.x (note I added the change to the REAME manually - I mised it in the last patch)
Comment #8
mkalkbrennerSee #496650: Unclick links don't work in 6.x-1.x-dev 2009-Jun-19