Since the bef_checkbox() function is responsible for generating actual HTML output, it makes sense to make it a theme function that can be overridden through normal means.

CommentFileSizeAuthor
#11 1404656_better_exposed_filters_checkbox_reroll_again.patch2.51 KBcmonnow
#8 1404656_better_exposed_filters_checkbox_reroll.patch2.44 KBbogdan.racz
#1 1404656_better_exposed_filters.patch3.6 KBLes Lim
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Les Lim’s picture

Les Lim
he/they
Primary language English
CreditAttribution: Les Lim commented
Status: Active » Needs review
FileSize
1404656_better_exposed_filters.patch3.6 KB

Patch attached.

Les Lim’s picture

Les Lim
he/they
Primary language English
CreditAttribution: Les Lim commented

This is related to #1344650: BEF for taxonomy terms with image fields

Status: Needs review » Needs work

The last submitted patch, 1404656_better_exposed_filters.patch, failed testing.

kingandy’s picture

kingandy CreditAttribution: kingandy commented

Is there a particular reason this function doesn't use theme_checkbox and theme_form_element_label to generate its HTML?

kingandy’s picture

kingandy CreditAttribution: kingandy commented

... Actually, there's no need to call theme_form_element_label separately, theme_form_element will add it automatically if we just add the appropriate #title and #title_display properties to the $properties array.

le72’s picture

le72
Location Yerevan 🇦🇲
CreditAttribution: le72 commented

So, do we have any solution?

le72’s picture

le72
Location Yerevan 🇦🇲
CreditAttribution: le72 commented
Status: Needs work » Needs review

#1: 1404656_better_exposed_filters.patch queued for re-testing.

bogdan.racz’s picture

bogdan.racz CreditAttribution: bogdan.racz commented
Issue summary: View changes
FileSize
1404656_better_exposed_filters_checkbox_reroll.patch2.44 KB

Rerolled the patch

zmove’s picture

zmove CreditAttribution: zmove commented

+1, very basic need. Solid patch, 7 month old... should be commited ASAP.

cmonnow’s picture

cmonnow CreditAttribution: cmonnow commented
Priority: Normal » Major
Related issues: +#1826582: Make bef_checkbox() a proper theme function so it can be overridden, +#1660322: Make field labels themable.
cmonnow’s picture

cmonnow CreditAttribution: cmonnow commented
Priority: Major » Critical
FileSize
1404656_better_exposed_filters_checkbox_reroll_again.patch2.51 KB

I've rerolled the patch again and set the issue to "critical" since it seems the suggested changes (on the surface at least) will have few, if any, serious side effects on existing installations yet will greatly simplify theming requirements (and more importantly, let developers override checkbox theming the expected/Drupal way).

mikeker’s picture

mikeker CreditAttribution: mikeker as a volunteer commented
Priority: Critical » Normal

@cmonnow: Please keep some perspective: feature requests are rarely, if ever, major and are never critical. I appreciate the reroll and will review it as soon as possible.

mikeker’s picture

mikeker CreditAttribution: mikeker as a volunteer commented
Status: Needs review » Fixed

#11 with fixes for whitespace errors and an edited version of hook_update_N from #1 has been committed.

Thank you, everyone, for your work on this and your patience!

  • mikeker committed 004036b on 7.x-3.x 
    Issue #1404656: Added theme_rebuild to an update_N hook so in-place...
  • mikeker committed 3c8194d on 7.x-3.x
    Issue #1404656: Fixes whitespaces errors in https://www.drupal.org/files...
  • mikeker committed c90f290 on 7.x-3.x
    Issue #1404656 by Les Lim, cmonnow, rbmboogie: Turn bef_checkbox() into...

Status: Fixed » Closed (fixed)

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