I see that currently you are using a <br id="notify_clear"/> to clear the floats on the options, this would be fixed by the feature request i post here.

The idea would be to wrap the whole form on a <div id="comment-notify"> for example, and then in the CSS:

div#comment-notify .form-radios {
  overflow: hidden;
  heigh: 100%;
}

This would also mean that themers could theme the whole form, adding padding to the whole thing, margins, or give a specific styling to elements within this specific form. We could also use jQuery to open it nicely with a slideDown, or well, you get the idea. The thing is that it would then be targetable both in CSS and in jQuery.

I'm attaching a patch that gets this done, though be aware that this is my very first patch, I created it with Cervisia, so please let me know if the formating or whatever is incorrect.

Also, I'm not sure if this is the best way to wrap it but it does the trick on my test server... let me know what you think :)

CommentFileSizeAuthor
comment_notify-divwrap-css.patch1.75 KBManuel Garcia
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

greggles’s picture

Title: Wrap the comment-notify form elements in a DIV (incl patch) » Wrap the comment-notify form elements in a DIV and remove needless br
Version: 7.x-1.x-dev » 5.x-2.x-dev
Status: Active » Patch (to be ported)

@Manuel Garcia - thank you! This patch is wonderful. As you probably noticed by my comment in the module file I didn't like this solution but was unsure how to make it better. This seems much better :)

I have committed this to the 6.x branch of code. Marking it for backporting to 5.x (assuming any of this code gets backported to 5.x...).

Two suggestions for the future:

1) When you submit the patch use the status "patch (code needs review)" in the issue.
2) When creating a patch for a contributed module, do it from the module directory directly instead of from the parent directory

Thanks again for this patch - I hope you will continue providing such good work.

Manuel Garcia’s picture

Awesome greggles :D

For the record, i tried using #sufix and #prefix on this, but somehow the submit buttons were getting pulled in the div, so I went with this solution instead.

Also thanks for the suggestions on making/submitting patches :)

mattiasj’s picture

Very nice! Are there any plans for a next release (non-dev) where this will be included?

Manuel Garcia’s picture

this patch is already in the latest stable 6.x release mattiasj, though im not sure about 5.x since i'm not using it

greggles’s picture

Status: Patch (to be ported) » Fixed

A better solution is provided in #487820: Cleanup of comment_notify code so we should not port this to 5.x

Status: Fixed » Closed (fixed)

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