Hey,

Can anyone explain what's the rationale for setting the visibility to hidden in the following lines of CSS code:

.field .field-label-inline {
  visibility:hidden;
}

in content.css? Will I mess something up if I set it to visible?

Thanks a lot.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

Status: Active » Fixed

It lets you have nicely aligned

My label : value 1
           value 2
           value 3

Independantly of the lenght of 'My label' :-)

removing it will turn the display to :

My label : value 1
My label : value 2
My label : value 3
ahoria’s picture

Priority: Normal » Minor

Cool, thanks.

I just found this doesn't work well with the Content Taxonomy module.

Anonymous’s picture

Status: Fixed » Closed (fixed)
eMPee584’s picture

Title: CSS: hidden inline field » CSS: hidden inline field is a bit unexpected, add option for visible inline
Version: 5.x-1.6-1 » 6.x-2.x-dev
Category: support » feature
Priority: Minor » Normal
Status: Closed (fixed) » Needs review
FileSize
1.56 KB

The current behaviour does not apply well to the new multigroups. This patch adds another option that displays all labels inline. The CSS is changed in a way that causes no change after applying the patch.

markus_petrux’s picture

@eMPee584: Could you please try this one? I believe it fixes the issue with the multigroup module. If it works, then I could include this change in the patch for #196421: Deleting unwanted multiple values / multiple values delta issues

If it doesn't cover what you meant, then I think I missunderstood something. :-o

eMPee584’s picture

Status: Needs review » Needs work

So you believe, eh? *g
Well no, it doesn't work and if it would it's still lame to give each and every label a '-first' class. My patch works but at the cost of introducing another option 'inline-all' that needs to be set for multigroup labels. i chose that to make the change non-intrusive but instead checking if the field is within a multigroup should be a better way. imho the CSS should still be changed like my patch does.

markus_petrux’s picture

I'm afraid that changing the CSS will affect existing sites, so there might be a backwards compatible way to fix the labels for fields in multigroups.

Maybe touching content_preprocess_content_field() adding information related to the group the field belongs and then extending the template files array to cover group variations so we could provide separate content-field-multigroup-tpl.php, etc ?

eMPee584’s picture

the patch i posted *is* backwards compatible that's what is stated in the comment aswell!

markus_petrux’s picture

FileSize
361 bytes

Patch in #4 is changing the meaning of CSS classes, and that may potentially affect themes/sites that override them. Also, we've been changing content-field.tpl.php, which again it may affect existing user base of CCK.

How about adding something that is only related to the multigroup module?

The attached patch adds the following.

.multigroup .field .field-label-inline {
  visibility:visible;
}
markus_petrux’s picture

Status: Needs work » Needs review
eMPee584’s picture

Well while i agree with your reasoning and like the simplicity of the patch, it simply does not work if the group is not displayed as a fieldset because.. well there is no surrounding fieldset in that case!
Well back to the point about changing the CSS: for one CCK has constantly been changing APIs and i think everyone using it right now and especially with fieldgroups is aware of possible breakage and able to manage it. IMHO the decision to have the property visibility:hidden apply to the class field-label-inline was not the best decision anyways. That's why i renamed that in the original patch. The template should really check if the field is in a multigroup and skip the -inline-first logic if in a multigroup... however in the variables passed from content_preprocess_content_field i could not find a multigroup marker. That is what i'd propose, change the CSS for sake of sanity and make the template detect the field is within a multigroup.

However there is one catch: there might be use cases for the original behaviour of displaying only the first inlined label.. in that case, my previous patch would be the best solution..

markus_petrux’s picture

...CCK has constantly been changing APIs and i think everyone using it right now and especially with fieldgroups is aware of possible breakage...

Maybe, but I guess it is not desirable to keep changing things, specially since the launch of CCK 2.0 is supposed to be stable enough for sites. CCK is the 3rd most used project, after Drupal itself and Views, with at least a bit more than 15,000 sites using it. See CCK usage stats.

IMHO, If you're going to break backwards compatibility, you need a reason to. So, I'm trying here to propose alternatives that could work with respect to existing user base.

If there's someone running CCK 2.1 on a live site in production, upgrading should be as simple as possible, a matter of just uploading files and running update.php. Everything should work without further changes, unless there's a good reason. That's my opinion, of course.

Anyway, the way to go is something that CCK maintainers have to decide. I'm fine with any solution as far as the multigroup module can be launched because the site I'm working on needs this, which is way I've spending time working on the multigroup module.

...well there is no surrounding fieldset...

Oops, that's probably something that should be fixed in the multigroup module. Not only because thi issue, but also because a wrapper for subgroups is necessary in case someone wishes to style subgroups.

However there is one catch: there might be use cases for the original behaviour of displaying only the first inlined label...

Maybe it would be nicer than adding 'inline-all' if theme/content-field.tpl.php had some information about the context of the field? That would allow themers style it differently when a field is part of a fieldgroup / multigroup and would cover what you said here.

yched’s picture

Just chiming in :
"CCK has constantly been changing APIs and i think everyone using it right now is aware of possible breakage." isn't true. While things have been in the moving during the (quite long) 6.x dev, alpha and even RC phases, we're striving *very* hard to not break or change existing behavior while in an 'official release' cycle. This has been the case in the D5 branch, and will be in the D6 branch as well. And this in fact makes a large share of the complexity of CCK maintainance ;-).

Sorry for not having the time to be more constructive on the issue right now. Big thanks to both of you for your time on the CCK queue.

eMPee584’s picture

Ok i admit that was a bit polemic but the real point i wanted to make is that anyone who has modified CCK templates should be qualified to adapt to minor CSS changes easily. It's not even an API thing ;)
However, a surrounding

would also fix the CSS you posted...
however i still strongly feel the inline class with visibility=hidden should be renamed to inline-hidden changing the css from
 .field .field-label-inline,
 .field .field-label-inline-first {
   display:inline;
 }
 .field .field-label-inline {
  visibility:hidden;
 }

to

 .field .field-label-inline,
 .field .field-label-inline-first,
 .field .field-label-inline-hidden {
   display:inline;
 }

 .field .field-label-inline-hidden {
   visibility:hidden;
 }

with the template adapted to use

.field-label-inline-first
.field-label-inline-hidden
.field-label-inline-hidden
.field-label-inline-hidden

for normal multiple value labels and

.field-label-inline-first
.field-label-inline
.field-label-inline
.field-label-inline

with multigroup field labels. For the sake of sanity. Giving all labels in a multigroup the class .field-label-inline-first is just wrong.

markus_petrux’s picture

Status: Needs review » Fixed

Well, I think I found a solution that doesn't require any patch to anything else but the multigroup module. Please, check out comment #241 in the following issue:

http://drupal.org/node/119102#comment-1158186

In short: the stylesheet shipped with the multigroup module includes the following:

/* Inline field labels visible within the content of multigroups. */
.multigroup .field .field-label-inline {
  visibility:visible;
}

The class "field-label-inline" will be used for all inline labels in multigroups. There's no need to fix the field template and anything. It's just resolved within the multigroup module. All subgroup containers use now the class "multigroup", but also "multigroup-x", where X is the delta value of the subgroup.

Setting this issue as fixed. Feel free to reopen if you think this still needs some kind of amendment to the content module. Otherwise, please use the multigroup related issue.

eMPee584’s picture

well that you solved the issue without changing the CSS has it's advantages but i still think the CSS classes should be renamed like reasoned for in #14.. anyways gonna check it out and report back.

Status: Fixed » Closed (fixed)

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