It'd be nice to be able to override the output of the terms of use in case you have a SUPER long one, so the registration form doesn't end up being 50 pages long. Currently, #value is hard-coded in the form, so you'd need to do another form_alter to change it, which is not very themer-friendly.

CommentFileSizeAuthor
#3 terms_of_use-themeable-264016-3.patch2.1 KBwebchick
#1 terms_of_use-themeable-264016-1.patch1.91 KBwebchick

Comments

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Active » Needs review
StatusFileSize
new terms_of_use-themeable-264016-1.patch1.91 KB

aaaaand a patch. This keeps the same behaviour, just moves it to a theme function.

You'll need to clear your cache w/ the button @ admin/settings/performance to see the results.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Oh, btw I don't really know why I'm passing in $node. It seemed like a good idea, just in case you wanted to add a class to the div based on some flag on like the node ID or something.

Can you tell I don't theme? ;)

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
StatusFileSize
new terms_of_use-themeable-264016-3.patch2.1 KB

OR!

We could do this, which makes it in a nice scrolling div.

Btw, this is my first CSS patch ever. I'm a little frightened.

Chill35’s picture

Chill35 commented

Yeah the Terms should be scrollable... They are bound to be long, and boring to boot... ;-)

I will apply the patch thank you. I will test it, play around a little bit with it first. I wonder if the checkox can be disabled until one totally scrolls down the Terms, use jQuery to find out if the bottom of the Terms are visible...

In the last patch, class 'content' and 'clear' are gone for the Terms I think... I think this is needed. These classes are always used for the node wrapper div.

I also would like to have some ajax attach to the Node field on the admin settings page, so that a check is made to see if the node exists, by loading it, and do a form_error if the node is non-existant, and while I am it, I may provide a preview of the Terms on that same page. Sounds like a lot of stuff, but this might be really easy, with the super FAPI functionality, and require a small number of lines... and show off a great functionality. It's not auto-complete I will use here, something else.

Since I will do a node_load on the Admin page, might as well show a preview of it, and have it styled the way it would/will be on the registration page.

Speaking of loading the node, it's same with theming... I will pass the $node to the theming function... it's already in memory, so why not pass it along ...? Yep, yep.

Yep, clear the cache to clear the Registr-y after register-ing a theming function, thank you for the reminder.

Thanks a lot, Angie.

I still have to write the tutorial too...

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

I wonder if the checkox can be disabled until one totally scrolls down the Terms, use jQuery to find out if the bottom of the Terms are visible...

Sure, that's a great idea.

In the last patch, class 'content' and 'clear' are gone for the Terms I think... I think this is needed. These classes are always used for the node wrapper div.

D'oh. Yes, I removed those while I was debugging thinking they might be causing the problem and forgot to add them back when things were working.

I also would like to have some ajax attach to the Node field on the admin settings page, so that a check is made to see if the node exists, by loading it, and do a form_error if the node is non-existant

+1 on the node title autocomplete. That'd be much better from a usability standpoint. I was going to try to figure that out and submit a patch, but I thought this one was a bigger win (since you only do that node ID thing once), and only had so long to work on it. ;)

and while I am it, I may provide a preview of the Terms on that same page.

Sure. I don't feel strongly one way or the other on this one. I could see it being useful though.

All of these should likely be handled as separate issues though; the fact that you can't theme the output is kinda a bug. :)

Chill35’s picture

Chill35 commented

All of these should likely be handled as separate issues though;

Yes.

the fact that you can't theme the output is kinda a bug. :)

Oh ya.

Chill35’s picture

Chill35 commented
Status: Needs review » Fixed

I decided not to add a css file after all. Not for now.

There are so many CSS files added here and there, by module x and module y.

There is a 'id' hook now (thanks to your latest patch) added to the 'div'... so a CSS rule can be added to style.css to make that div height fixed _and_ scrollable.

That way it is more respectful of the theme.

I probably will change my mind tomorrow ;-)

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Yay! That's good enough for me. Thanks. :)

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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