I'm not sure there's a need for a "bold this text" option if the "'Read more' text" field allows HTML. Instead of (un)checking a box, the user can either add a <strong> element around the text or apply font-weight: bold to the .read-more class in their theme.

Alternatively, there could be multiple formatting changes -- say, bold and italics -- inside a fieldset. It just feels strange to have only one formatting option.

We are considering this module for the new Drupal.org theme, and we'd like to use the opportunity to make some minor streamlining tweaks. This is a very useful module!

CommentFileSizeAuthor
#1 ed_readmore_markup_improvements.patch1.22 KBtodd nienkerk

Comments

todd nienkerk’s picture

todd nienkerk commented
StatusFileSize
new ed_readmore_markup_improvements.patch1.22 KB

The attached patch simplifies how markup is wrapped around the text. It also provides an example of how other formatting options can be easily added.

mcurry’s picture

mcurry commented

I agree that it's not necessary. I prefer using CSS to control the appearance, which was the original intent of this module. Let's remove it.

mcurry’s picture

mcurry commented
Assigned: Unassigned » todd nienkerk
Status: Active » Needs work

@Todd, since I've granted you CVS access, please work up a patch that doesn't have any markup (other than a theme function's default CSS class). (Or, if you have other suggestions, please make them.)

todd nienkerk’s picture

todd nienkerk commented

We should add an upgrade path that (1) detects if the user had checked "Bold text," (2) wraps the stored variable in <strong> tags if needed, and (3) deletes the variable. This will need to be part of the same patch.

mcurry’s picture

mcurry commented

We should add an upgrade path that (1) detects if the user had checked "Bold text," (2) wraps the stored variable in tags if needed, and (3) deletes the variable. This will need to be part of the same patch.

That's a nice touch. The feature was exposed only recently, so I'm not sure how many users might have chosen such an option. Feel free to go ahead and work up a patch if you wish; otherwise I'll have to tackle it when I have time. (Probably not for a week or two.)

todd nienkerk’s picture

todd nienkerk commented
Version: 6.x-3.0 » 6.x-3.x-dev
Status: Needs work » Fixed

Bold text option has been removed. An update function has been supplied to behave as outlined in #4 above. Committed to 6.x-3.x-dev.

todd nienkerk’s picture

todd nienkerk commented

EDIT: Accidentally posted a reply to a different issue. Please ignore this comment.

Status: Fixed » Closed (fixed)

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