Great module!

Wanted to add no-repeat but nothing happened. Noticed you'd got your variable name wrong in one place and a semicolon was needed so here's my first ever attempt at submitting a patch...

CommentFileSizeAuthor
#5 913376.patch728 byteshaza
noggin.patch848 bytesAnonymous (not verified)

Comments

eaton’s picture

eaton commented
Status: Needs review » Needs work

Actually, I'd originally intended the extra-attributes field to set the no-repeat and other related flags on the background image. Hard-coding the semicolon would make that impossible. You CAN manually add a semicolon to the field and it works to chain extra properties, but I'd prefer to maintain the ability to set the repeat and so on. Am I misunderstanding the implications of the patch?

Anonymous’s picture

Anonymous (not verified) commented

There's actually two parts to the patch:

1. Fixing the incorrect variable name (from noggin:attributes to noggin:extra_attributes) - this is needed whatever the choice re part 2.

2. To use the 'short' version you could just add extra attributes, but with the semi-colon you can just add the long version, e.g. background-repeat: no-repeat, so it will still work. I guess it's up to you, I just didn't find it necessarily easy to understand, but I'm not really front-end dev. Perhaps just more instructions would help here if you don't want to alter the code.

Jeff Burnz’s picture

Jeff Burnz commented

IMO we just need to get that variable name right, which would be real good it that was fixed.

Maybe the description needs some minor work to clarify what and how to enter background properties. As a front end dev I would see "attributes" as things added to html elements, in CSS I would only refer to an attribute in terms of a selector (attribute selectors), strictly speaking CSS has declarations, properties and values - in this case we would be adding properties and values, such as no-repeat 0 -100px.

Maybe we need something more like:

    $form['noggin']['settings']['extra_attributes'] = array(
      '#type' => 'textfield',
      '#title' => t('Additional CSS properties'),
      '#default_value' => variable_get('noggin:extra_attributes', NULL),
      '#description' => t('Add a space separated list of CSS background properties such as no-repeat center.'),
    );
eaton’s picture

eaton commented

Thanks for the clarification, Jeff. I'm about to roll a new release and I think the changes you described will help clarify the purpose and behavior.

haza’s picture

haza
Primary language French
commented
Status: Needs work » Needs review
StatusFileSize
new 913376.patch728 bytes

Just fixing the missing variable name here, because I think this really should be corrected as this is a real bug ;)

hmartens’s picture

hmartens commented

hellooo..anybody there? Is this module still being maintained? Eaton you said December that you're about to roll out a new build but still nothing. A year later and still no resolve for this..

Please help!

Jeff Burnz’s picture

Jeff Burnz commented
Status: Needs review » Fixed

Fixed by default in #1002758: Allow each theme to have unique header images

Status: Fixed » Closed (fixed)

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