Patch adds more context to the theme. No API change.

CommentFileSizeAuthor
block-title-more-context.patch577 bytesamitaibu

Comments

cpliakas’s picture

cpliakas commented
Status: Needs review » Needs work

I can definitely see where this would be useful. Marking as "needs work" for the minor nit-pick that the array key should be facet_name instead of facet for consistency with the nomenclature used throughout the rest of the module. In other areas, facet_name is used for the machine readable name of the facet, whereas facet is always the array containing the definition returned by facetapi_facet_load(). Again, a very minor change, otherwise this patch is RTBC and will be integrated in.

Thanks for the contribution,
Chris

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented

> facet_name is used for the machine readable name of the facet, whereas facet is always the array containing the definition returned by facetapi_facet_load()

Actually in the patch, I am passing a full array that holds also the facet itself. Do you wish I pass only the facet_name so others will have to facetapi_facet_load()?

We should probably make sure in follow up patches, that we always pass the same context (e.g. facet_name) to all the theme functions.

cpliakas’s picture

cpliakas commented
Status: Needs work » Reviewed & tested by the community

I'm sorry! I misread your patch. If it is passing the full facet definition, then that is fine by me. I think passing the full array is better anyways, because you have more information to use and eliminate the need to call facetapi_facet_load() in theme functions. I agree with you that for consistency in follow-up patches we should always pass the same context, i.e. the full facet definition.

cpliakas’s picture

cpliakas commented
Status: Reviewed & tested by the community » Fixed

Committed with a few minor tweaks at http://drupalcode.org/project/facetapi.git/commit/7519833.

Thanks, and congrats on becoming the 17th committer to Facet API.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.