Hi, I wonder if this thing is necessary:

$link.before(checkbox).before(' ');

The issue related to this part of code is in that any CSS applied to input and link would be separated by this extra space and probably broken. Attached is example of possible issue.

If this space could be removed?

CommentFileSizeAuthor
#6 facetapi-1374048-6.patch1.09 KBcpliakas
space-issue.png3.27 KBcr0ss

Comments

cpliakas’s picture

cpliakas commented

Hi cr0ss.

I'm not sure. This is a code snippet that was adopted from Apache Solr Search Integration, so I would have to check as to why this code is there. Let me bump Peter and check what the purpose of this code is and whether it could be removed.

Thanks,
Chris

cpliakas’s picture

cpliakas commented
Title: Extra space » What is the purpose of the extra space added before checkboxes by the facetapi_checkbox_links widget?

Changing title for clarity.

cr0ss’s picture

cr0ss commented

Thank you very much indeed cpliakas. I guess if that code is unnecessary it would be great to remove it, so that any developer could use simple CSS to theme that block.

pwolanin’s picture

pwolanin commented
Title: What is the purpose of the extra space added before checkboxes by the facetapi_checkbox_links widget? » Extra space

Well, you need some space otherwise the checkbox and link text run together. I think this was Robert's markup originally, but it would seem a non-breaking space is the right thing to use if the checkbox and link are supposed to stay together?

cpliakas’s picture

cpliakas commented
Title: Extra space » What is the purpose of the extra space added before checkboxes by the facetapi_checkbox_links widget?

Re-setting title.

cpliakas’s picture

cpliakas commented
Status: Active » Needs review
StatusFileSize
new facetapi-1374048-6.patch1.09 KB

Would something like this work? Instead of padding via a non-breaking space, it pads via CSS. Or is there some accessibility reason why a non-breaking space would be better?

cr0ss’s picture

cr0ss commented

cpliakas, I agree with your patch. Removing hardcoded space and adding CSS gives users more flexibility to set specific spacing between checkbox and label they want.

pwolanin’s picture

pwolanin commented
Version: 7.x-1.0-beta8 » 7.x-1.x-dev

Does that work? It would seem you need to use class facetapi-checkbox-processed

cpliakas’s picture

cpliakas commented

It does because the margin is being applied to the checkbox that is added via JS which has the facetapi-checkbox class. The original link has the facetapi-checkbox-processed class which the CSS in the patch is not acting on. An example of the markup in a list item is copied below:

<li class="leaf last">
  <input type="checkbox" class="facetapi-checkbox" />
  <a href="/search/site/Aliquip?f[0]=bundle%3Apage" rel="nofollow" class="facetapi-checkbox-processed facetapi-inactive">
    Basic page (295)
    <span class="element-invisible">Apply Basic page filter</span>
  </a>
</li>
cpliakas’s picture

cpliakas commented
Title: What is the purpose of the extra space added before checkboxes by the facetapi_checkbox_links widget? » Remove the non-breaking space added before the links in the checkbox widget and replace with CSS padding
Category: support » task
Status: Needs review » Reviewed & tested by the community

Marking as RTBC. I am going to let this sit a few days in case anyone has any further objections.

cpliakas’s picture

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

Committed at http://drupalcode.org/project/facetapi.git/commit/3ce2027.

Status: Fixed » Closed (fixed)

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