Problem/Motivation

Node teasers include a Read more link to the full post in the node links, which are displayed in a row under the post in most themes. Users consistently report that this position for the Read more link is not intuituve and end users do not notice it. A more natural position for the link would be the end of the post's main text.

There are existing contrib modules which add this functionality:

However, for usability reasons, we would like to make the position of the link configurable in core.

Proposed resolution

  • Make the position of the Read more link configurable, with options something like the following:
    (o) At end of summary
    ( ) In links section
    ( ) Both
    ( ) Do not display
  • When this option is set to "At end of summary" (or "Both"), append the Read more link to the end of the teaser using DOM manipulation. (String parsing or regexes are not recommended; see #72.)
  • Make the link text themable, so that the rendering can be customized.

Remaining tasks

Several aspects of the feature are still under discussion:

  • The exact text of the form option labels.
  • What type of form element to use (radio buttons, checkboxes, or a select box).
  • Whether to include the "Both" option.
  • Where the configuration form element should appear (see #203).

The most recent patch for this issue, in #180, is nearly two years old. A lot has changed since then, so the patch needs to be rewritten, with the following changes:

  1. Update the patch to use the field API, and provide the configuration option for any Long text with summary fields.
  2. Update the theme function to use the render API (see #170).
  3. Incorporate final decisions for the points above.

User interface changes

  • Current appearance of 'Read more' link in Drupal 8: 'Read more' link in Drupal 8
  • New feature with new configuration form element.

API changes

None.

Original report by chrisada

That is the behaviour of most systems, and I think it is more natural.

Right now, the 'read more' link are often overlooked, especially when there are many other items in node->links. (comment, trackback, stat, del.icio.us tags, etc.)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

factoryjoe’s picture

FileSize
185 bytes

+1 on this. I actually had to hack this for the CivicSpace site, and it's still buggy, but much better than having it hidden down with the node links.

I've attached my code for doing in this in PHPTemplate.

moshe weitzman’s picture

if you prefer, this could be done with nodeapi('view') in a module instead opf in the theme.

chrisada’s picture

But is there an easy way to remove the link from node->links once we've added it in somewhere else?

I like the quick fix in #1. What kind of bug does it suffer from?

factoryjoe’s picture

FileSize
3.12 KB

The bug might be simple to fix, but I'm not sure how to do it... Basically you get an extra divider character in the node links. See the attached screenshot (there's an extra dot next to "Add new comment").

David Hull’s picture

I agree with this feature request also.

To make it easier to notice that a story was continued, this is the change I made in version 4.5.2:

--- node.module.orig	Tue Jan 18 19:55:29 2005
+++ node.module	Tue Jan 18 19:56:02 2005
@@ -524,6 +524,10 @@ function node_prepare($node, $teaser = F
   }
   else {
     $node->teaser = check_output($node->teaser, $node->format);
+    if ($node->readmore) {
+      $node->teaser .= " [...]";
+      $node->teaser = preg_replace(',(</p>\s*)( \[\.\.\.\])$,i', '$2$1', $node->teaser);
+    }
   }
   return $node;
 }

The preg_replace is there to attempt to move the "[...]" inside the last paragraph.

chrisada’s picture

Version: x.y.z » 5.x-dev

Updating applicable version to see if there is still interests in this.

Gurpartap Singh’s picture

The paging.module(5.x release) has example of overriding "Read more" link. http://drupal.org/project/paging

Gurpartap Singh’s picture

Version: 5.x-dev » 6.x-dev

The ed_readmore.module gives you this feature. But this is on feature wish list for 6.x too.

Pancho’s picture

Version: 6.x-dev » 7.x-dev

Though I really like this proposal, we unfortunately won't get it into D6...
For D7 we might think about rehauling the whole links system, as it is not flexible enough at present.

SeanBannister’s picture

Hey Pancho, I'm really interested in this feature as I think it's a huge usability improvement for Drupal. Hae there been any issues posted for the potential links overhaul? I couldn't find one.

mcurry’s picture

*subscribe*

I support this as well. I rolled MerlinOfChaos read more hack into a module (ed_readmore) which I use on every site I operate. I've found that the standard read more link placement is one of the most confusing things for site users who aren't 'Drupal aware'.

Anyway, there's an issue filed against the ed_readmore module : #290905: Should "Read More" tweak be in core for Drupal 7. I'd really rather have this functionality in core for a variety of reasons. So, how may I contribute to this effort?

mcurry’s picture

Some goals or features I'd like to suggest:

  • Read more link HTML generation should be 'themeable' (rendered in a theme function). This would allow changing the "Read more" text on a per-site basis without resorting to core hacking or using the locale module.
  • The default theme function should render the read more link HTML wrapped in a stable, easily-selected CSS class (class="read-more" or similar) to simplify CSS-only appearance tweaks.
SeanBannister’s picture

Hey inactivist,
As the maintainer of ed_readmore did you want to role a patch against core, I was just looking at Pancho's post and noticed it was 8 months ago so there hasn't been much activity around this lately, I'd be happy to help you with a core patch.

psicomante’s picture

+1 to this feature!

maartenvg’s picture

@all:
1. is it your opinion to include all features of ed_readmore.module into core (e.g. inline on/off, GUI for editing 'read more' string)?
2. And do we need to leave the original behavior intact, and thus putting an on/off button on ?q=admin/content/node-settings?

My opinion is:
1: no, the inline should be the only behavior because that has the best effect. The GUI is not necessary if we use a theme-function.
2: I'm not entirely sure, in my eyes the current behavior is not good enough to stay, but it's not a big problem to make it optional.

I would like to mention that since d7 has dropped support for PHP4, and with it the need of providing our own strripos() function. :)

BTW. I'm almost finished creating a patch. I only need to implement the theme function.

maartenvg’s picture

Status: Active » Needs review
FileSize
5.24 KB

I've added a patch with the following features:
- the logic to place it at the inline location. Doing this in core is way, way simpler than doing it via hook_nodeapi!
- option at admin/content/node-settings to influence the position of the 'Read more' link (either inline or with the other links)
- themeable (when positioned inline)

What still has to be done.
- catch other block-type tags
- improve wording of admin/content/node-settings
- add tests to node.test

I'll wait with the above improvements until I've had some response.

psicomante’s picture

i agree with maartenvg. Great patch.

Only a PS: in my installation of ed_readmore.module, if i put "break" tag after a ul/li list, the "read more" link appeared before it and not after the list.

SeanBannister’s picture

Will test patch soon,
I agree there's no need for a GUI. Moving the Read More link just makes sense.

thomas23@drupal.org’s picture

subscribe and +1 to get it out of node->links

maartenvg’s picture

#19: To get this into core it has to be tested by as many people as possible.
Please test this patch. More information about patching (if necessary) see http://drupal.org/patch and http://drupal.org/patch/review.

Dave Reid’s picture

FileSize
4.25 KB

Patch applied and worked as desired, but I had a few problems after looking at the code:
1. I don't agree with the logic re-write in node_links(). It should be combined like the pre-patch condition. If down the road the node.module needs to add another link, we'd have to re-arrange this back the way it was.
2. Why is str_replace(' ', '&nbsp;', t('Read more')) needed in theme_node_read_more_link()? We don't do the same thing in node_link(). Spaces in nodes teasers or bodies are not replace with &nbsp, so why should we do it here?
3. Do we need to implement "TODO: Need to look for any tag allowed by the current filter." from _node_read_more_link()?
4. Do we really need constants for a simple 0/1 setting? If there's a possibility that there will be future placements for the "Read more" link I'm for it, but is that something foreseeable?

Overall, this is a nice usability feature for normal readers. Node readability is improved when the link is inline. I know before I started using the ed_readmore module that people had a very hard time understand there was more to a post since the "Read more" link in the nodes. I literally had to write at the end of the posts, "Hey if you want to see the rest, click the 'Read more' down below!" Not very friendly. +1 for getting this in.

I've attached a patch that addresses issues #1 and #2.

maartenvg’s picture

Status: Needs review » Needs work

ad 1) I agree, your solution is more elegant.

ad 2) The text "Read more" or a translated version of that, shouldn't break onto multiple lines, because that influences the recognizability and readability of the "Read more" link, decreasing its usability. By using non-breaking spaces, we prevent the link to break. This restriction is not necessary for the links provided by node_link().

ad 3) Yes. It's a annoying bug that it doesn't work properly when the teaser ends in a non-

block like a div, list or blockquote.

ad 4) It's not really necessary, I think. I do not think there will be another placement method, but if so, we can add the constants then. I vote to remove them.

So I'd like to see the above patch improved by restoring 2), fixing 3) and removing the constants per 4). If I can find the time, I'll work on that today.

Dave Reid’s picture

Status: Needs work » Needs review
FileSize
3.96 KB

New patch with some changes ready for some testing.

1. maartenvg, thanks for your explanation of str_replace(' ', ' ', t('Read more')) and did my own tests to confirm this. I added a short documentation that describes why that needs to happen.
2. Found a list of block-level tags from _filter_autop and now instead of just searching for </p>, it searches for the last, ending block level tag. I think this completes the TODO feature.
3. Removed constants.

maartenvg’s picture

Status: Needs review » Needs work

Looks great, but let's make some tests for this, because it has a history of breaking over unexpected content. I'm working on that now.

maartenvg’s picture

Hmm. I've look a bit at how the block level tags are performing, and it seems that it just isn't quite what we're looking.
For example, a teaser ending in:

The next topic

becomes

The next topic Read more

(there's a link in there)

Which definitely isn't what we want. Also, there are some tags in there where we don't want to use, like <hr /> and <thead>

Before we can proceed, we need to discuss which tags we need, and what behavior should be connected to that. For example: a blockquote is always of the form <blockquote><p>CONTENT</p></blockquote> if "Convert line breaks" is turned on, so we need to look for that explicitly. Some of these are also dependent of the "Correct broken HTML" setting of the input formats.
I'll start:
Ending in ...</p>: append before end tag
Ending in ...</h1-6>: append after end tag (on next line)
Ending in ...</p></blockquote>: append before end tag
Ending in ...</blockquote>: append before end tag
Ending in ...: append after end tag (this is the seldom case where the teaser ends without a tag, when all input formats are turned off)
Ending in </div>: before
Ending in </p></div>: before
Ending in </pre>: after
Ending in </address>: before (will alter the style of the tag, but probably not too much)

How to handle lists, tables and forms?

SeanBannister’s picture

Just wanted to keep this going because we really need this in Drupal 7. It seems that Drupals usability often lacks a lot but in very simple ways and this is a step in the right direction.

I'm just trying to get my head around your last post maartenvg, I'm not quite sure what the problem is, could you explain it a bit more in detail so I can brainstorm it.

maartenvg’s picture

The problem is that a teaser not always ends neatly with a </p> or </div>. Sometimes it's a </h1>, </ul> or </pre> or no tag at all, et cetera. The current implementation will place the Read more link before almost all known block elements, making the look and placement of unpredictable and sometimes even will produce non-valid HTML, like </li><a href="#">Read more</a></ul>. For example, the latest patch will transform a teaser which ends in <h1>This is a heading</h1> into <h1>This is a heading <a href="#">Read more</a></h1> which looks like this

This is a heading Read more

Not quite what we had in mind?

So we need to determine which block level tags need which behavior: before or after the last tag(s).

SeanBannister’s picture

Ok, I see. Thanks for clarifying, I hadn't quite got my head around it.

I agree with the tags you mentioned. For tables and forms we'll just have to append after and it will need to sit on the next line.

Here's a list of all HTML 4 Elements that we will need to think about http://www.w3.org/TR/REC-html40/index/elements.html
Maybe we should setup a Wiki page so we can work together on the list???? I presume using one of the Drupal Groups, any suggestions?

mcurry’s picture

Finding the appropriate place to insert the inline teaser is definitely a challenge (see ed_readmore module's issue queue, if you have any doubts.)

See: #190948: Tweak confused by <ul>, <ol>, etc, #164343: Read More link needs to insert itself before more tags that could end a teaser

Does the output format (input filter) being used on a node have an impact? I think it might.

Perhaps the core implementation should have an admin-defined list (with a default), or perhaps a set of regular expressions, so that no core-level hacking is necessary in order to address breakage in the field.

Also, should the administrator have the option of disabling/enabling this for RSS feeds?

dugh’s picture

I support this, too. Although for me the main issue is not so much the placement of 'read more' but the text itself. It is a vague phrase. An action verb is better. See: http://www.copyblogger.com/click-here/

Click through rates:

"Read more": 1.8%

"Continue to article": 3.3%

"Click to continue": 8.5%

So as long as this patch allows people to customize the teaser text, too, that would be great.

Until then, we are using the ed_readmore module which allows changing this text.

ericjam’s picture

how do i implement this in 6x? how do i do this patch thingie?

rfay’s picture

Please do add this feature! It's an excellent one and should be in core.

SeanBannister’s picture

Issue tags: -Usability
SeanBannister’s picture

bsherwood’s picture

+1 Subscribe!

This should definitely be in core. Almost ALL CMS systems handle "read more" like this.

I am using ed_readmore and I noticed a two issues with it that I hope doesn't happen if this gets into core.

1. If the <!--pagebreak--> happens to be just after a heading tag,(i.e paragraph, heading, pagebreak, paragraph) the 'read more' link will attach itself to the last <p> tag. It looks unnatural.

2. When used with the feedburner module, the path doesn't follow correctly. When you hover over the "read more" link, the path will be correct except the base URL. The base URL changes to http://feedburner.google.com/. So if you have a node at: http://www.example.com/content/test it will show up on feedburner as http://feedburner.google.com/content/test. Obviously not the desired result.

geerlingguy’s picture

Issue tags: +Usability

Simply agreeing with dugh - we should have the text be customizable; I like to use "Continue Reading »" for some situations, "Read more" for situations when a posting shoots the user off to another site, and "Full Article »" for magazine-style sites.

dddave’s picture

Subscribing!

momper’s picture

+1

mean0dspt’s picture

+1

3duardo’s picture

+1

jgossage’s picture

+1
when the end tag issues are resolved

Todd Nienkerk’s picture

+1. Excellent!

_nolocation’s picture

+1

Todd Nienkerk’s picture

Please note that Read More is again under active development for inclusion in the Drupal.org upgrade and retheming. Changes are afoot!

funana’s picture

+ 1

Todd Nienkerk’s picture

Regarding the patches added by Dave Reid and inactivist's comment #29, please see #164343: Read More link needs to insert itself before more tags that could end a teaser for the red flags associated with inserting links into most elements. For example, the link cannot be added in the midst of <ul>, <ol>, and <li> elements because it results in invalid markup.

David_Rothstein’s picture

This looks like a really good patch in the works.

A couple things:

1. It sort of "feels" to me like the decision of where to put the read more link belongs in the theme layer - for example, see http://api.drupal.org/api/function/template_preprocess_node/7 and how it splits off the taxonomy links. I wonder if there's a way to do this one similarly?

2. Does it really have to be the job of this patch to figure out how to deal with HTML that gets cut off between tags? I'm not sure it does. Drupal has an HTML Corrector filter that serves this purpose, and the "read more" link should append perfectly fine to any content that is rendered using that filter. If someone is using a text format that doesn't contain that filter, by contrast, then they are already going to have all sorts of problems with HTML getting chopped off and rendered in weird ways, regardless of whether the read more link is in there or not.

3. Check out #372743: Body and teaser as fields for an interesting issue that might be related to this one. If the node body just becomes a regular old field, and if any textfield in Drupal can be "teaser-enabled", then maybe the 'read more' link is something that gets appended on a field-by-field basis? That's probably thinking a bit ahead, though :)

jackalope’s picture

+1

Gurpartap Singh’s picture

Too many +++s there. I counted, they are 13. Someone should now fold their sleeves and sit with a cup of coffee.

Anonymous’s picture

*subscribe*
not interested in the technical details (i´ll leave this to the experts), but from the users point of view, the current default "read more" system is simply nuts.

Todd Nienkerk’s picture

Title: move 'read more' links to end of node content » Move Read More link to end of node content
Status: Needs work » Closed (duplicate)

@David_Rothstein: I'm marking this issue as a duplicate of #164343: Read More link needs to insert itself before more tags that could end a teaser and have copied your comment (#47) there.

David_Rothstein’s picture

Status: Closed (duplicate) » Needs work

That's a useful link, and seems worth coordinating efforts with that, but since this issue is about moving the feature into D7 core and that issue is about the D6 module, it seems this one shouldn't be marked as a duplicate?

Dave Reid’s picture

Yes, this should be left separately since it applies to getting the module functionality into Drupal core.

Todd Nienkerk’s picture

David_Rothstein and Dave Reid: Yikes! Sorry about that. I totally forgot the scope of this issue is not the Read More module. Must've been half-asleep.

babbage’s picture

+1. (Still no coffee.)

Cool_Goose’s picture

+

arnd’s picture

I know this issue also from another cms and several users have complained about that.

bendshead’s picture

+1 yup

zzlong’s picture

I support this request, kind of must-have enhancement for Drupal.

jbc’s picture

+1

saccard’s picture

yes, should be a Drupal7 feature

degure’s picture

I also would appreciate more flexibility for the "read more" link. Thanks for considering it.

MGParisi’s picture

Yes! Agreed... cant live without Read More...

May I add that maybe... I am ok with just Read More...
Would love it if it worked with the common "Comments" also... If you have a Comment, you also have more to see, so therefor should "Read More..." be displayed..

I guess more control over these fundamentals would be nice.

Aveu’s picture

I use this module and I find it vital but I think the correct approach would be to address this as an integrated part of the D7 teaser content control (see http://drupal.org/node/274947) AND make the "footer" much more configurable in general.

UPDATE: Per comments made (see replies #91-97) below I am re-editing this comment to indicate that while I support inclusion of this feature my reason for not working on it is that I do not have the skill to do so.

reglogge’s picture

all for it
+1

Duplika’s picture

+1

axbom’s picture

+1 this is always one of the first things I have to fix on a new install

mkrakowiak’s picture

subscribing

Todd Nienkerk’s picture

In case anybody is curious, there's a release candidate for Read More link 6.x-5.0. It's a major overhaul that includes many new features and customizations.

Gerald Mengisen’s picture

+1

danfinney’s picture

+2 (my wife thinks so too)

Damien Tournoud’s picture

Please avoid all kind of string parsing. That's ugly and fragile. It should be easy, fast and robust to do that using DOM manipulation.

kevcol’s picture

I'd like to see this in Drupal 7. Is much more intuitive and far easier than workarounds.

noovot’s picture

+1 definitely

morleti’s picture

+1! absolutely yes! and great thanks to developers who implement it!

yraffah’s picture

+1

jorsol’s picture

It must be a feature in Drupal 7...

+1

zooki’s picture

THANK YOU +1

iamwhoiam’s picture

Don't know why its not already in!

webchick’s picture

Because people keep posting totally unhelpful things like "+1" and "Don't know why its not already in" rather than actually working on the patch.

geerlingguy’s picture

@ webchick - lol, the last patch attempt was way back in #23. Since then there are at least three people complaining about the lack of effort, but plethora of +1's. I don't have time to figure out much for a patch right now, but I would be more than glad to test one if someone else makes it.

cazador’s picture

Should be in core

superfly’s picture

+1 should be in core. this is one of the modules I *always* install.

webchick’s picture

Status: Needs work » Closed (won't fix)

OK. Obviously no one here is serious about seeing this issue fixed. Feel free to set back to "needs review" when you have a patch attached.

Gurpartap Singh’s picture

webchick, +1.

MGParisi’s picture

@webchick

I know the frustration you feel about everyone wanting something, but no one providing it. I have had to deal with the issue in the Doc Queue, and it can be quite irritating. At times I want to get into my car, drive to the Airport, fly to the individuals home city, rent a Cab, and then take a bag of dog poo and light it on fire on their door step.

So instead of continuing to frustrate you (or anyone) I will offer a trade. I can not write the patch, I am simple not the person to have the ability to work with other peoples code. I barely know the API, little less the core... My solution... I will TRADE anyone willing to work on this with a task I can perform. Be it support or documentation for a project that is important to the individual who has the ability to write this code.

MGParisi’s picture

Sorry, forgot to add the following XML tags

<sarcasm>first paragraph</sarcasm>
<unusualsolution>Second Paragraph </unusualsolution>

Note Proper use of XML is important :-p

Rob C’s picture

+1 this has to be in core.

geerlingguy’s picture

I think the reason people keep +1'ing this, even though they don't want to help, is because the 'Read more link' project page tells them to do so... http://drupal.org/project/ed_readmore

Is Todd Nienkerk around? Can he maybe try to get this thing tied off? It'd be nice to have it in core, but it ain't gonna happen by typing +1.

MGParisi’s picture

Edited beyond original post.

MGParisi’s picture

There are a ton of people who COULD work on this patch (around 8X or so). I feel that the question no one wants to bring up is: Why do 80 some odd people feel uncomfortable or unable to contribute a patch for core?

Ohh and one simple but not complete answer to this question is probably the "wont fix" status. It was set to "wont fix" because a person with significant Drupal Clout got angry and basically closed the issue. I think many people who may be interested in working on this issue will see this status and simply move on. I think attitude for contributing to Drupal Core is a key aspect to this discussion.

Example: I know that Mikey_P published a module that I wanted from his sand box. I was to maintain this module (and have been). While working on the modules main page, an individual came in and basically told us that the module was crap (BTW, I maintain this module without acknowledgment on my profile). This individual is another one of those Drupal "Leet". Sorry, but I will contribute where I do not have to take heat for my work. I want to help out, but I am backed up into a corner. My situation is complex (to complex to get into it here), so if you really care to change things, then lets put aside pride, ownership and feelings and have an open (separate) discussion addressing the issue.

ericjam’s picture

Priority: Normal » Critical

Not to bust balls but the ed_readmore module has saved my life. Maybe you could take a page from them.

yoroy’s picture

Priority: Critical » Normal
Status: Closed (won't fix) » Needs review

re: #91 I agree the won't fix status might scare off would-be contributors.
Let's see what the next 7 responses will be and re-evaluate this issue at comment #100 :-)
Back to needs review because there is a patch somewhere. It's not critical though.

Status: Needs review » Needs work

The last submitted patch failed testing.

espirates’s picture

+2

Damien Tournoud’s picture

The last real status of the patch in #23 (changed in #24) was "needs work". Plus, I already indicated (in #72) that the regexp approach was a no-go.

Aveu’s picture

MGParisi: I can't speak for everyone but while I definitely appreciate this module and want to see some version of it make it into Drupal 7 core (see my comment above #64) I simply do not have the skills to work on it. I am just a volunteer webmaster for a small nonprofit and I spend most of my energy for their website just trying to keep the content the way they want it. Drupal is a blessing for that kind of work. Unfortunately while I want to learn the guts of Drupal I can only put in a small fraction of my free time to learning the internals and it's a big learning curve for me -- I have never been a webmaster before so I have been simultaneously learning PHP, SQL, and CSS -- I'm an just an old programmer learning new languages and systems. :)

I wonder if folks would go back to their earlier "+1" posts and RE-EDIT them to include why they don't / won't work on this. I have stated my reasons but I will be the first to re-edit my original post just to get this started. See http://drupal.org/comment/reply/16161#comment-1487126

webchick’s picture

No, please don't re-edit your posts. That only bumps this issue in the tracker, pinging all 500 people to look at it, only to discover that there is once again no patch attached, and the issue is no closer to being resolved. It wastes valuable time, and we're into the last 6 weeks of code thaw and need all the time we can allocate spent on issues that are actually going somewhere.

I appreciate that not everyone who's in favour of this feature has the means to work on it, but there is no point in wasting everyone's time explaining this, nor adding yet more utterly useless and aggravating "+1s" to the pool. People want this feature. We get it. What we need is someone to work on the patch, as has already repeatedly been explained.

So if you can't help this feature along with working code, then it's best not to reply at all, unless it's to offer help in other means, such as offering to test it once someone eventually, maybe, someday, uploads a patch.

Sigh.

webchick’s picture

Oh. I should point out though that there is no one in the entire Drupal community, including Dries, who is paid full-time to work on Drupal core. Each of us is volunteering our own time, and are balancing our core development work with the same "real world" constraints as all of the the people requesting that this issue be fixed. Believe it or not, core developers also have jobs, family, household chores, volunteer work, and are trying to maintain some semblance of "a life". We therefore need to prioritize our time, and tend to do so around issues we ourselves need fixed.

Additionally, none of us were born Drupal/PHP experts; we all learned core development by "scratching our own itch" on some problem that happened to bother us, making mistakes, trying again, and getting better over time. I got started in Drupal core development by monkeying with forum stuff. Others by fixing typos or documentation. Still others with CSS and markup fixes. There is an endless supply of places to jump in.

This hopefully will help explain why no amount of "+1s" will ever fix this issue, and why it only serves the exact opposite of its intended effect, as the implication is that you're ordering around people who are volunteering their time for free, and demanding that they solve a problem for you that you are either unable or unwilling to solve yourself.

At the end of the day, what we really need is someone with both the "itch" and the will to do what's necessary to scratch it. That's it.

dugh’s picture

The code's in the ed_readmore module, which solves this issue

kaakuu’s picture

My 2 or 3 cents would be -

#
Make this templatable OR determinable by the site admin from work flow.
For example $read_more can be placed at the end of $teaser in continuity OR placed at the beginning or end of $link

#
There is apparently a difference between teasers and excerpts and accordingly two different positions of "Read more" link may be needed according to the $teaser used by a site. There is also subtle differences in Read More, Read Full, Read details and this differences may need different positioning of the further reading link.

#
Sites that have been using the Read more link in $link and do want to use that way should not forced a lack of compatibility.

geerlingguy’s picture

Status: Needs work » Closed (won't fix)

Okay, even though we all have great hopes and dreams for this thread, it's going nowhere. As they say in my home state, "Show me the money"—make a patch or move along to an issue where you can help move things along. Status is won't fix (and should stay that way) until someone contributes a patch. Please!

MGParisi’s picture

Status: Closed (won't fix) » Needs work

Geerlingguy, let the thread stay in the Queue. I understand the frustration people have over no one contributing code, however like Webchick said... some of us have lives and RIGHT now I am busy in life (Im getting Married and Moving). I am looking to dive into core dev and this maybe my intro to the project. If I am to late to include it in 7.x I will try to include it in 8.x! Anyways, closing the thread prevents people from seeing (or goggling) the issue in the queue and possibly contributing code.

We all have to grow up. If you want it build it. If you don't like that no one builds it then don't worry, someone will Just be patient.

I personally havent worked on core development because many of the ideas I express in the IRC channel or issue in this queue are largely ignored. If I am the only one that has interest in a feature then why should I work on a patch to so that I can contribute it and then have it ignored. At least in this thread I (or anyone else) has a great support base. It is also a great simple change that would be perfect for someone who wants to start working with core!

I must admit that I am bit frustrated people demand a status be set when so many others disagree with his decision. Also I am a bit upset that the guy who wrote the original module started this petition when instead he should of coded the patch!

SO LEAVE THE STATUS ALONE! Some Queue issues can stay open for many months (or years) without becoming invalid!

MGParisi’s picture

Also, there is a very good dialog going on here about how the feature should be implemented.

also

I have gotten frustrated by this situation too, but I have found that confronting it only makes things worse. It looks bad on Drupal and the Open Source community. Open Source does have a flaw. If no one works on an issue it either never gets implemented or it takes time and community support for it to catch the attention of anyone willing to work on it!

geerlingguy’s picture

The thing is, this issue has been on the issue queue for three different releases of Drupal now (5.x, 6.x and 7.x), and since 2008, there has been quite literally no work on it. Lots and lots of +1's (myself included), but no work.

I'm not going to touch the status anymore, but I know that every time I see "1 new" in my personal tracker on this issue, I get excited—"maybe someone's finally going to do something," I think to myself—but every time I click through and view the post, I see the same, boring, mind-numbing thing: +1. (I think that's also why webchick was a bit miffed at this issue in #84).

If you'd like to work on this, more power to you—post a patch, even if incomplete. There are many around who don't know a dime's worth of PHP but could at least test your patch and offer feedback :-)

Augusto Ellacuriaga’s picture

+1 must be in the core

mr.baileys’s picture

Status: Needs work » Needs review
FileSize
8.77 KB
24.34 KB

Well, I wanted to add a +1 to this thread, but didn't dare doing so without at least some contribution, so here goes:

  • A lot has changed since the last patch was added to this thread, especially when it comes to node rendering. That, added with the fact that string parsing and regexes should be avoided (#72, #96), I decided to start from scratch, although I've used the patch in #23 as well as http://drupal.org/project/ed_readmore as inspiration.
  • A major difference with earlier patches, and the "Read more" module, is the fact that this patch treats the placement and configuration as part of the text field formatting instead of the node module. David_Rothstein introduced this idea in #47-3, and I think it makes a lot of sense. The "read more" text can be set per field instance, so it could be "read full article" for articles, "view blog post" for blog posts, etc...
  • Although configurable in this patch, currently only the <p>-tag is accepted when it comes to placing the read more link inline in the summary. If the summary ends in any other tag (<div>, <blockquote>, ...), the read more link is appended after the ending tag instead of inserted before the ending tag. Reading through #164343: Read More link needs to insert itself before more tags that could end a teaser, this seemed to be the only viable option.
  • The read more link is themeable (as requested in #12).
  • I had a chat with dbabbage on IRC, and we agreed that a user should at least be able to revert to old-style
    "include-read-more-in-links" to provide backwards compatibility. I have added a "Read more link" section to the "Display" fieldset for each content type, where the location of the read more link can be set:
    • hidden: no link at all
    • inline: inline in the summary (default)
    • link: as a regular link (backwards compatible with D6)

    Screenshot of the new portion attached, wording might need some tweaking... Also, for some reason, switching
    between the options doesn't immediately take effect (caching issue?). Still looking into this...

  • Although it's also possible to add a text field to configure the text of the "read more" label through the interface,
    I'm not sure if this wouldn't crowd the interface too much.

Setting to CNR so the testbot can take a look...

Status: Needs review » Needs work

The last submitted patch failed testing.

mr.baileys’s picture

Status: Needs work » Needs review
FileSize
8.77 KB

Fixed an error in node_link() that caused this patch to fail.

geerlingguy’s picture

I like it - just to be clear... is this something administratively set per-node, or per-content type? I would rather see it per-content type, but if there was an option to have it either/or or both/and, that would be best (kind of like how the meta tags module works) - because I wouldn't want just anybody choosing where the read more links go...

Sorry I can't patch right now; will try this later today if possible, while updating a couple other patches.

babbage’s picture

Status: Needs review » Needs work

Installation
Applies to current Drupal HEAD.

Testing undertaken:
Using a fresh installation of the current HEAD, created three Page nodes that was longer than the trimmed summary length, and promoted them to the front page. Experimented with the three options for the "Read more" links. All looked good and seemed to work as expected. Placement of Read more link inline looks good. (I believe there may have been one occasion when I attempted to change from one link placement and it did not stick, but not sure now—this may however be related to below...) Then added an Article node similarly set up, and set the Read More link preference for that node type to be different to the Page nodes. No cacheing was active on the site.

Errors

  • At times, changes in the Read More preference were reflected immediately, but at other times changes did not result in any change, even after manually flushing the cache (which I tried despite cacheing being turned off).
  • With the Page node set to links, I set the Article node to inline. This resulted in an inline Read More link for the Article node but the Page nodes then had Read More links both inline and in the links section. Changing the Article setting, deleting the article, even deleting the Article node type did not change this.
  • Subsequently setting the Page node type to display no link resulted in both links disappearing. However, turning back on Inline links resulted again in both links re-appearing.

Conclusion
I have not explored all the combinations and permutations here, and it may have nothing to do with the Article node type being set to a different link type. However, something clearly isn't quite right. It is probably quite minor, but needs to be tracked down.

More general comments

  • Liking where this is going... this must be close. The inline placement of the links is exactly how I'd want to see it.
  • You have my 200% agreement :) that inline should be the new default.
  • I wondered whether admins might want the ability to set the default preference that would apply to new content types that are created, rather than it defaulting to inline. However, thinking about it I don't think there is the possibility currently to set other new-content-type defaults on a site-wide basis at present so no reason this should be any different.
  • I wondered whether a select list would be better than radio buttons but perhaps inside the collapsing fieldset it doesn't matter so much. It takes up a bit of space but this is the content type configuration screen after all—not something you have to revisit every day.

Well done for actually writing a patch on this ancient issue! Now let's get it in D7 at last... :)

mr.baileys’s picture

Status: Needs work » Needs review
FileSize
9.12 KB

@geerlingguy: currently, this is set per content type, not per node. Allowing it to be overridden on a node-by-node basis is an interesting concept, but might be best tackled in a follow-up issue once this lands or through a contrib module...

@dbabbage: thanks for the in-depth review. The error you noticed was caused by a timing issue while saving the content type: the node's fields were reconfigured before persistent variable changes were saved. Since the teaser read more link location setting is stored in a variable, the new location didn't take effect until right after the content type was saved (with the old location setting still in effect). This has been now been fixed.

I wondered whether a select list would be better than radio buttons but perhaps inside the collapsing fieldset it doesn't matter so much. It takes up a bit of space but this is the content type configuration screen after all—not something you have to revisit every day.

I modeled this after the "preview post" section in the submission form settings fieldset, which also uses radio buttons for the three options. On the other hand, this would be easy to change into a dropdown if you think that's more appropriate.

kaakuu’s picture

Is this http://drupal.org/files/issues/read_more_0.png exactly going to core ? If so Cheers!!

geerlingguy’s picture

@ mr. baileys - sounds good; the most important place would be the content type anyways.

The jury's out for me, whether it would be better as a select list or radio buttons... but I'm inclined to say stick with the buttons, since it *is* inside a fieldset, and therefore takes up the same amount of room when collapsed.

webchick’s picture

Wow! mr.baileys, thank you for restoring my faith in humanity! :) Now let's hope that 1/10000th of these +1ers are willing to actually put some effort in to do the work necessary to review this patch and get it to RTBC. http://drupal.org/patch/apply has a lot of tips; feel free to drop by #drupal on IRC and ask for help if you get stuck.

Adding some relevant tags. Haven't looked at the patch.

SeanBannister’s picture

Just tested the patch. Two of the three options work well :
- "Do not display a read more link for this content type"
- "Display the read more link in the links section, underneath the trimmed post"

However when I use "Display the read more link inline as part of the trimmed post" the link isn't displayed at all.

Also I seem to randomly recieve the following error message when I change between the 3 options:
Warning: DOMNode::appendChild(): Document Fragment is empty in theme_field_formatter_text_summary_or_trimmed() (line 343 of /web/www/drupal7/modules/field/modules/text/text.module).

This error has appeared on a number of pages:
- Content Types page (example.com/admin/structure/types)
- When I edit a Content Type (example.com/admin/structure/node-type/article)
- And on the home page where the node is being displayed

Thanks for your work on this patch, I'll keep testing.

babbage’s picture

@SeanBannister: I originally saw similar text.module errors but the disappeared after running update.php, which I neglected to do after updating to the latest HEAD. Could that be the issue?

mr.baileys’s picture

Issue tags: -Needs screenshots
FileSize
9.41 KB
100.2 KB

I've attached a screenshot of what the end result looks like for each of the three settings (inline links, no links, old style links), and this for three types of articles (a normal article, an article with the summary value defined, and an article with a long list, for which only the first items are shown after text trimming)...

Screenshot of the interface was attached in #107.

I've also updated the patch with 2 sanity checks, in case the generated summary is empty, which should resolve the issues reported by SeanBannister in #116 (although I haven't been able to reproduce this error on my end)

@SeanBannister, can you check and see if this resolves the issue you are seeing?

SeanBannister’s picture

FileSize
88.68 KB

I just performed a fresh install of the latest HEAD and applied the #118 patch, works perfect.

I've tried pretty hard to break it. Switched between the settings a few times and created nodes with and without summaries. Used a few different tags that I thought might break things <blockquote> <p> <ul> <ol>.

I think from a usability point of view an ellipsis "..." after either the content or the "Read more" would really improve things but only when "Display the read more link inline as part of the trimmed post" is selected. I think we'd have to place it after the "Read more" not after the content because we'd have issues when we use <li>'s. I've attached a screen shot example.

SeanBannister’s picture

Just another point on usability, I'm just thinking the descriptions might be a little bit long and confusing for new users.

I'd like to use the "Preview post" section as an example (Located on the edit content type page).

Preview post
() Disabled
() Optional
() Required

This is very simple and doesn't repeat itself.
Currently the "Read more" section is a bit more confusing :

Read more link
() Do not display a read more link for this content type
() Display the read more link inline as part of the trimmed post
() Display the read more link in the links section, underneath the trimmed post

I would propose we simplify it a little :

Read more
() Disabled
() Display in node summary
() Display in links section

I'd appreciate some feedback on this.

moshe weitzman’s picture

Good lord. That DOMDocument code in test.module is a bit much for core IMO. Would be good to get some opinions on that.

mr.baileys’s picture

FileSize
6.62 KB

@moshe: I agree that it does look a bit unwieldy... Don't know if we can tone it down a bit more, but I do agree with earlier comments in this thread that DOM Manipulation is more reliable than (and preferred to) regex hacking.

Regarding the placement in text.module instead of node.module: my reasoning was that the read more link and the summary are so closely related, that it made sense to include the logic for the read more link in the formatter. It would also make it possible to use this on other objects besides nodes, like users for example. On the other hand, besides adding bloat to the otherwise relatively clean text.module, it also introduced a problem of building the actual link.

Because of the reasons above, I decide to create another patch, this time with the logic in node.module.

@SeanBannister: I've improved (shortened) the wording on the interface as per your suggestion, although I did leave the caption (read more link) in place. I like the idea of an ellipsis, but I think this should part of the logic that does the 'trimming'. This way, ellipsis can be added if text is broken up mid-sentence, and it could be dropped when text is broken up between paragraphs for example. Might be a separate issue though...

New patch attached, the only thing I'm not happy about is the left part of the following statement

$node->content['body']['items'][0]['#post_render'][] = <snip>
Damien Tournoud’s picture

Yay for proper DOM manipulation. This looks really awesome, mr.baileys.

Status: Needs review » Needs work

The last submitted patch failed testing.

babbage’s picture

Rather than:
Read more link
( ) Disabled
( ) Display in node summary
( ) Display in links section

I would prefer:
Read more link
( ) Disabled
( ) Append to node summary
( ) Display in links section

(Italics just to draw attention to the changed section—I'm not suggested it is italicised, obviously.)

yched’s picture

(Thread is very long, so maybe this has been raised earlier)
Problem is it's fairly difficult to know when there is 'more' to see on a node. It's not just related to whether the body has a summary different than the full text.
There's more to see:
- if *any* '"text with summary" field displayed in teaser mode has a summary or a trimmed version different than the full text,
- more generally, if there are fields visible in 'full' mode that are not visible in 'teaser', or if a field is visible in both but using different formatters ('summary or trimmed' on teaser and 'full text' on full node).
So, pretty hairy...

Damien Tournoud’s picture

Yves, I'm sorry but you are being out of topic here. This issue is just to give the opportunity to concatenate the "Read more" link, when it displayed, to the end of the node content. This issue doesn't try to change the rules that govern the visibility of the "Read more" link.

SeanBannister’s picture

@dbabbage: "Append" is perfect, I knew there was better way to say it :)

Any word on the patch having "10 exceptions"? I'm keen to get this in by the 1st of September so I'm ready to patch when I've got the go ahead.

yched’s picture

@damZ #127: OK, got your point, but my remark is not completely OT.
The 'append to node body summary' option is not really suited to a world with fields (body being only one of them).
- The option is confusing if $node_type->has_body is FALSE (and the current patch will probably raise warnings since $node->content['body'] is undefined in those cases)
- 'Body' might very possibly not be the only field for which 'append readmore link' can make sense.

In fact, I think mr.baileys' #122 is spot on :"Regarding the placement in text.module instead of node.module: my reasoning was that the read more link and the summary are so closely related, that it made sense to include the logic for the read more link in the formatter. It would also make it possible to use this on other objects besides nodes, like users for example. On the other hand, besides adding bloat to the otherwise relatively clean text.module, it also introduced a problem of building the actual link."

The 'append readmore link' makes inherently more sense as a formatter setting IMO.
2 problems with this:
- there's currently no UI for formatters settings right now in the Field UI in CCK HEAD or the patch in #516138: Move CCK HEAD in core. This will need to be adressed at some point anyway, but it makes moving forward on this issue right now a bit complex. I think making it an instance setting, as in patch #118 is perfectly fine meanwhile.
- "problem of building the actual link": Quite true - we'll need to solve that for imagefield in core and 'taxonomy as field'. See #525622: 'as link' formatters need a generic way to build the url of an 'entity'

Gurpartap Singh’s picture

+1

babbage’s picture

So... this is being set on a per-content-type basis. Taking into account yched's suggestion that limiting this to the body is not appropriate in a world of fields, where the body is even optional, what about adding to the admin interface a select box to specify which field the "Read more" link should be attached to, for each content type? Doesn't sound like it would be difficult...

mr.baileys’s picture

Status: Needs work » Needs review
FileSize
8.78 KB

Setting this to NR for the testbot.

@dbabbage:

Taking into account yched's suggestion that limiting this to the body is not appropriate in a world of fields, where the body is even optional, what about adding to the admin interface a select box to specify which field the "Read more" link should be attached to, for each content type? Doesn't sound like it would be difficult...

I think yched was referring to the really ugly hack of targeting the body field ($node->content['body']['items'][0]['#post_render'][]). This was just temporary until I figured out the best way to target the fields from node.module, but since I've moved the code back to text.module, this no longer is an issue. I'm pretty sure this line was also responsible for the exceptions, as yched indicated.

I think for the time being, we don't need an admin interface to configure which field to add the read more field to. Once the Fields UI allows configuration of formatters, we can take care of that, but meanwhile the current approach should be acceptable IMO. In the event that this behaviour is unwanted, users can still revert to putting the link in the links section or disable it altogether.

yoroy’s picture

- We're not using the word 'node' anymore in the ui. Would 'Append to summary' suffice? Think so.
- re 131, 132: agree with mr.baileys we don't want to create a per-content type solution here before we have a Fields UI. Let's beware of scope creep, esp. with an issue that needed 100+ mostly useless comments before getting up to speed.

Oh, and thank you very much mr.baileys for taking this on, much appreciated!

yoroy’s picture

Status: Needs review » Needs work

status…

mr.baileys’s picture

Status: Needs work » Needs review
FileSize
19.18 KB
8.96 KB

@yoroy, "append to summary" would certainly suffice, new patch attached. Also included an up-to-date screenshot of the UI.

SeanBannister’s picture

Tested the patch and couldn't break it.

SeanBannister’s picture

When a user previews a node before submitting it should we display the "Read more" link in the trimmed post preview if it's displayed inline???

babbage’s picture

Re #133, I agree with what was said at #132 and my comments in #131 were me confusing the issue in my head, but note that this patch is *already* a per-content-type solution, as it currently stands. Just sayin'. :)

yoroy’s picture

dbabbage: all good then :-)

mr.baileys’s picture

Issue tags: -Needs tests
FileSize
12.74 KB

Added tests...

mr.baileys’s picture

gentle nudge... Would be great to get this 4 1/2 year old issue resolved...

SeanBannister’s picture

Tested #140 patch, works great.

jeffschuler’s picture

Tested #140 on a clean install of D7 HEAD.
Used Devel Generate to create Page and Article content. All "Read more" links appeared properly for both content types at different Trim lengths, and in both positions, (appended and in links.) I have not yet tested with additional fields, richer HTML content, or inline images.

A thought: instead of the 3 radio buttons, how about two checkboxes:
[X] Append to summary
[ ] Display in links section

And allow the link to be put in both locations. Disabled state is inherent when neither is checked.

"Read more" in the links section is, I think, standard only to Drupal, (and questionable, as we're suggesting,) but having a "Permalink" in the links section is fairly common.
Maybe:

[X] "Read more" appended to summary
[ ] Permalink in links section

I also wonder if "Read more", (appended,) should only be printed when there actually is more to read. That if the post has not been trimmed, (it's short enough, or length set to Unlimited,) the link shouldn't be printed. With the modified choices I've described, the Permalink would always appear in the links section, but the "Read more" only when the post has been trimmed...

moshe weitzman’s picture

+++ modules/field/modules/text/text.module	17 Aug 2009 09:24:02 -0000
@@ -744,3 +799,7 @@
+function theme_read_more($link, $inline = FALSE) {

Is there some reason that we can't use our standard render() model to append the read more link? This string/xml concatenation is exactly what we are trying to get away from. Think of $page. This technique does themeing at time of node build. We are now deferring theming until drupal_render_page().

+++ modules/field/modules/text/text.module	17 Aug 2009 09:24:02 -0000
@@ -744,3 +799,7 @@
+function theme_read_more($link, $inline = FALSE) {

I think this should be named theme_container or somesuch. It should take an $attributes array and 'class' => 'read-more' should be passed in.

mlncn’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs usability review

Huge usability win. Established by many over years of changing this on every site that really matters to them!

And the patch works! D7 is looking sweet these days too.

Two notes, which are separate issues and don't have to be resolved by this patch, but I'd be interested in following up on as separate issues:

  • Read more link is present even when everything is already included in the teaser. I know there should be clickthrough for (leaving) comments, but "Read more" when there isn't more to read feels wrong.
  • I really like to have ... and then the Read more link when it is in inline mode.
moshe weitzman’s picture

Status: Reviewed & tested by the community » Needs work

i asked a couple questions right before Ben.

mlncn’s picture

Status: Needs work » Needs review

(I this tab open too long and didn't see either Jeff's or Moshe's posts before finally submitting-- I like the changed options that Jeff suggests, and Moshe's points too-- back to Needs Review, at least, but the Usability review remains checked off!)

mlncn’s picture

Status: Needs review » Needs work

Cross-posted again. I quit.

SeanBannister’s picture

I really like the check box idea :
[X] Append to summary
[ ] Display in links section

But I'm unsure about "Permalink" I know a number of CMS's use the Permalink but the majority of users have no idea what it means and the users who do understand what it mean also understand what "Read more" means and understand that it serves the same purpose as the Permalink. I'm personally a fan of sticking with "Read more".

babbage’s picture

I like the suggestions for the changes in #143, but it is important to think about what "Permalink" means—because it means more than just "this is where main content page is right now". I don't think we should be hard-coding into core the suggestion of permalinks when many site administrators do not have a genuine commitment to maintaining URIs in the long-term (e.g. beyond CMS architecture changes on their site, which is what a "permalink" should mean)...

In other words, I am suggesting we need a different word as the default, like "Full Post" (probably same problem as "Read More") or "Post with Comments" (better but still problematic for obvious reasons if comments are disabled or there are none) or something. Suggestions?

yoroy’s picture

On

Read more link:
( ) Disabled
( ) Append to node summary
( ) Display in links section

Vs.

Read more link:
[X] Append to summary
[ ] Display in links section

The last one is more compact, but also increases mental load for the user. The 3 radio buttons communicate what they'll do unambiguously. The checkboxes make it the job of the user (instead of the software) to calculate what the setting will do. I suggest keeping the 3 radion buttons.

On the permalink: please keep the original scope of this issue in mind. I see how this is the same URL that could be re-used for this purpose but let's defer that to a follow-up patch and keep this about the read more link placement.

Bojhan’s picture

Lets work a bit on the Append.. that sounds rather programming-lingo to me.

ctmattice1’s picture

How about:

Read more link:
( ) Disabled
( ) Attach to node summary
( ) Display in links section

Somewhere in this thread someone commented that append sounded to programmatic, everyone can understand "Attach"

SeanBannister’s picture

@yoroy (#151): The reason I support the check box idea is the user "might want" the ability to place the Read More in the links section AND appended it to summary.

I also agree that the permalink concept is going to be a rather long conversation, lets keep it as a follow up issue/patch.

@ctmattice1 (#153): I like the fact that Append clearly states the link will be placed after the summary. According to the Oxford Dictionary:
append : verb add to the end of a document or piece of writing.
attach : verb 1 fasten; join. 2 include (a condition) as part of an agreement. 3 assign or attribute. 4 appoint (someone) for special or temporary duties. 5 Law, archaic seize (a person or property) by legal authority.

Bojhan’s picture

Thats the issue, you need the Oxford Dictionary to explain what it does. I like its exactness, but I dont think many people know that word.

SeanBannister’s picture

I disagree I personally believe append is a rather common word in the English language, maybe not so much if English is your second language. But lest face it append is much more common than Taxonomy which reminds me somewhat of the Taxonomy Vs Category debate around Drupal 5. The general consensus was we shouldn't dumb down the interface if it takes away from the meaning. I think the "exactness" of Append is exactly what we need.

:) I usually don't get so worked up over one word.

yched’s picture

+ 1 for 'append' vs 'attach'. I don't see the problem with "append". It's clear (add at the end) and common enough IMO, definitely not programmatic lingo. "Attach" has lots of other meanings and is thus much less precise.

dddave’s picture

+1 for append

Even as a non-native speaker I like append much better than attach. Even if someone doesn't know the exact meaning of the word attach still is very vague.

But lets face it: after one try everything should be clear anyways. So no need to dumb down the interface.

jeffschuler’s picture

+1 for append (vs. attach)

+1 for using Full Post (per @dbabbage in #150) instead of Permalink or Read More, when displayed in the Links section.
I was concerned, too, that Permalink is not exact or generally understandable, but I think Read more in the links section is clumsy... Full Post is more appropriate.

amc’s picture

Wait a minute..are folks trying to figure out what to put on the settings page or what the link itself should say? If it's the former, we don't really need a protracted discussion (not trying to squelch debate, just move things along). We can always change wording after code freeze. If it's the latter, why not just include a textfield setting and let the admin decide what the link should say?

jeffschuler’s picture

FileSize
14.4 KB

@amc: Point taken.

I've modified the patch to use 2 checkboxes instead of 3 radio buttons -- allowing for either, both, or neither.

I believe it works, though it doesn't pass its own Simpletests because I wasn't sure how to set the array value here in the Simpletest function, (lines 916-917 of modules/node/node.test) :

$edit['teaser_read_more_location'] = array('inline');
$this->drupalPost(NULL, $edit, t('Save content type'));

(preg_match() expects parameter 2 to be string, array given)...
I imagine the array needs to be serialized somehow?

@moshe: I realize we still need to attend to #144, though I think one of your code snippets there is meant to be something else...

@mr.baileys: Please feel free to jump back on this; I'm just trying to keep it moving.

mcrittenden’s picture

Subscribe.

yoroy’s picture

Status: Needs work » Needs review

re: append: it very much *is* a programming term, do a google search on it and you'll almost exclusively get results for programming tutorials, references etc.

'Add to end of' should be understandable by all

back to needs review for the latest patch

Status: Needs review » Needs work

The last submitted patch failed testing.

Bojhan’s picture

I am fine with fixing the text post code freeze.

babbage’s picture

yoroy: less than half of the top 10 Google terms relate to programming, and only one of the top five (#3). The rest refer to the usage we're using here. "Attach" conveys less information about what will occur. (Most people have at least heard of an "Appendix"?)

geerlingguy’s picture

+1 for Append

I also think it would be nice to allow users to theme the text for 'Read more' or 'Full post' - because different people use the teaser»full view link for different reasons.

yoroy’s picture

But let's not hold this up on wording… Append it is then. Somebody up for rerolling?

jeffschuler’s picture

Status: Needs work » Needs review
FileSize
14.61 KB

Re-rolled #161 with a working node.test.

moshe weitzman’s picture

To clarify, this is a render() structure that I am envisoning at end of the patch. This assumes that that theme_read_more is renamed to theme_container() and adds an attributes param.

$build['read_more'] = array(
  '#link' => l(),
  '#theme' => 'container',
  '#attributes' => array('class' => array('read-more')),
)
return $build;

Status: Needs review » Needs work

The last submitted patch failed testing.

yoroy’s picture

Status: Needs work » Needs review
yoroy’s picture

Status: Needs review » Reviewed & tested by the community

Functionally seems to have been ok. slightly abusing the rtbc status to see if we can get one last glance at this before freeze?

SeanBannister’s picture

We had 7 or 8 developers look at this patch at the Brisbane Code Sprint and we were generally happy with the way it looked. Discussed a number of different ways to implement this but kept coming back to this original patch. We tried to overcome the dom manipulation but quickly realised that it's very much needed.

We spoke with Moshe on IRC at the code sprint but I can't remember the outcome of the conversation.

Ended up not achieving a whole lot but being reasonably happy with the patch.

Status: Reviewed & tested by the community » Needs work

The last submitted patch failed testing.

Cliff’s picture

Subscribe

yoroy’s picture

Status: Needs work » Needs review

Sure bot?

mean0dspt’s picture

there's a code freeze coming. Don't you think it's time to commit this patch already?

yoroy’s picture

Status: Needs review » Needs work

Probably needs a reroll

jeffschuler’s picture

Status: Needs work » Needs review
FileSize
14.7 KB

Re-rolled to chase HEAD.

Status: Needs review » Needs work

The last submitted patch failed testing.

Cliff’s picture

I'm sorry, but from a usability standpoint I don't get the move from three radio buttons to two checkboxes. If you want the interface to work smoothly for everyone — even the newbies and people who might seldom use it — make the choices clear. Assuming that the user will figure out that they can leave both boxes unchecked to choose to display nothing is, well, assuming that they will be thinking about the interface and not about their choices. (If they are thinking about the interface, that's not good.)

As I understand it, one of the driving forces of the move to D7 is to improve usability. Often usability problems are obvious: A big blaring patch of red overwhelming the actual message, for instance. But the most insidious usability problems are like little tiny razor cuts that, individually, seem to make no difference while, taken together, they bleed the organism to death. This change is like one of those razor cuts.

I realize that with radio buttons we would lose the option of placing the link in two places — once before the summary and again after it. I can't imagine a use case where that would be a benefit, so it's hard for me to understand how that "loss" costs us anything. (Generally, people find it confusing when two identical or similar links that seem to do the same thing are fairly close to each other. Again, it's one of those things that gets them thinking about the interface instead of working with it.)

For what it's worth, my suggestion for the wording includes making the statement strong and active:

Place link to complete item:
( ) after summary
( ) with other links
( ) nowhere (I need no link)

Notice that the order is:

  1. the recommended location, based on the usability tests
  2. the current location, which some might wish to keep
  3. the choice the fewest people are likely to make
yoroy’s picture

This patch was RTBC before code freeze. Opening up the discussion around a broken patch after code freeze is not going to help this get into core. (I too am not happy with the checkboxes instead of radios).

Please consider handling wording changes etc. in followup issues after (if ever) this gets committed.

Cliff’s picture

@yoroy, one of the reasons code freeze was extended was to improve Drupal's usability and accessibility, right? I apologize for not having been able to participate more on this issue earlier, but I would hate to see us backslide where there was no usability problem to begin with.

Also, it occurred to me this morning that, if people really do want to be able to put the link in both locations, all that would take is one more button:

Place link to complete item:
( ) after summary
( ) with other links
( ) in both locations
( ) nowhere (I need no link)

The options are simple and explicit. The user is left with no second-guessing.

Having said this, I am sensitive to your concern that the current setup has you and other reviewers having to come look for more about a coding issue when this post has nothing to do with that. That is a real problem. I'm thinking about a suggestion and will add it as either a separate issue or a new comment in the issue about the issue queue itself.

babbage’s picture

I won't wade into the checkbox vs. radio button discussion. :)

I would like to suggest however that "Place link to complete item:" is ambiguous; "complete" can mean "full" but it can also relate to the verb "to finish"... as in, if you were working on a draft, this might be a link that you would click to "complete" it. I also think "place" is somewhat unusual in this context, as that verb refers more to the action of doing the positioning, rather than describing where the item will continue to be displayed... So "Display link to full item:" or better "Display link to full content:" would be more specific.

I'm also not personally very keen on "nowhere (I need no link)". I think "nowhere (do not display link)" would be clearer; we are then not getting into discussing the user's inner needs, just stating the effect of selecting the option. I would also prefer the simple "nowhere" over the currently proposed option.

Cliff’s picture

Those are good revisions, @dbabbage. I like it when someone takes an idea of mine and improves upon it!

Cliff’s picture

[How did that post twice?]

Everett Zufelt’s picture

+1 for radios instead of checkboxes. And also +1 for allowing users to modify text for link, even if only through the theming function.

Aveu’s picture

This is slightly off topic but definitely relevant. If there is any interest could someone move this post to another thread for further discussion? (I don't know how to do that other than creating a new one)...

READMORE LINK VISIBILITY

I mention this here because it depends on the administrative options menu that has been being discussed. I would like to suggest adding one more feature into the mix BUT it is a bit more theming related and so is probably out of the scope of this thread's issue (i.e.: placement of the readmore link).

The radio button idea that evolves from reading posts #184-#186 is good but I think something is missing. My suggestion is adding a related set of radio buttons which would only be active if either of the "with other links" options (radio button #2 or #3) are selected. The end result might look like this:

NOTE: The small footnote texts are just my explanatory comments for this post.

Place link to full text of item:
( ) after summary
(X) with other links
( ) in both locations
( ) nowhere (do not display link)

If there is no more text for any given item:
(X) disable and dim link for that item1
( ) disable and vanish link for that item2
( ) do not display link for that item3

1(link would be "greyed-out", using H/S/L calculations*)
2(link would become a placeholder, disabled and transparent)
3(this option may cause nearby links to move around a bit)

I am not sure of the best wording for these options and again if there is a desire to discuss then this post needs to be moved to it's own thread.

* formulas for RGB<-->HSL calculations:
http://www.easyrgb.com/index.php?X=MATH&H=18#text18
http://www.easyrgb.com/index.php?X=MATH&H=19#text19

yoroy’s picture

Version: 7.x-dev » 8.x-dev

Bummer :\

femrich’s picture

put off for drupal 8. unbelievable.

Dave Reid’s picture

@femrich: You are still more than welcome to enjoy the benefits of using a module, because you can still use those for Drupal 7! http://drupal.org/project/read_more

femrich’s picture

@ Dave Reid, Unfortunately, although there is a read_more project, there is no release (and hence no module whose benefits I might enjoy), nor does there appear to be lots of activity in the project given what's going on in the queue. (There's a call out for a co-maintainer for the project.) Unfortunately, I know nothing about coding or about creating modules. Otherwise I would put my energy into this one. So the reality seems to be that Drupal 7 is, for now, hobbled with illogical placement of the read more link except for people who can code the change for themselves. If anyone knows otherwise, I'd be happy to hear the solution...

RKS’s picture

I wish someone would explain the logic to me of why the Read more is treated like it is in core. It's been six years since someone brought this up in D5 and now this issue is updated to D8 because after all this time it still hasn't made it into core.

Obviously, if they don't care to fix this, they must have a reasoning behind it. I sure would like to hear why it is better for Read More to come after all other links instead of immediately following a teaser.

David_Rothstein’s picture

RKS: When you read through this issue, do you see anyone arguing in favor of the status quo? The reason it isn't fixed yet is because it's a hard problem to solve, and not enough people have actually stepped up to do the work to finish solving it. Instead, most people who have posted on this thread seem to be either subscribing or whining.

In the meantime, if there was ever an issue that needed a summary, this one is it. Adding tag.

(If anyone wants to help by doing that, see http://drupal.org/node/1155816 for instructions.)

RKS’s picture

Okay, well no need to start being an ass when someone asked for an explanation. I have just as much right to ask for some reasoning as you do for bitching and moaning that people complain instead of learning to be computer programmers. I love that argument. Yes, let's everyone become computer programmers and stop whining so being a computer programmer will mean nothing. You think there are still going to be people making money as a programmer if it were so easy to just take a weekend and learn some coding.

So again, stop having an attitude for no reason because it may be a hard problem for you or me to fix, but whoever designs the blog.module could probably figure out a different way of handling it if they wanted to because they know their module better than any of us. Obviously they don't care to. Maybe they have a reason they like it like that. That's what I was saying and just curious as to the logic behind it. I never said people here knew that answer. That's why I said things like "I wish" and "I would like to hear."

Sorry for being rude but I really don't appreciate being flamed for asking for some reasoning and it gets pretty tiring hearing all the high and mighty attitudes of some people who complain because Drupal is open source with tons of users and they get "annoyed" because many of those users aren't programmers by day and superheroes by night. I'm sure you know plenty and that's awesome but that doesn't mean you can talk down to people whenever you want.

mcurry’s picture

UN-subscribe :D

David_Rothstein’s picture

@RKS: Sorry, I didn't mean to imply that you personally were whining; I apologize if it came across that way. However, there has certainly been some whining on this thread.

It is simply not true that you need to be a computer programmer to help get this issue fixed. I never said anything of the kind. For example, the last round of comments on this issue were about hammering out what the user interface should look like (usability + user interface text), which has nothing to do with programming at all. And anyone who can read through this issue could write up an issue summary (or at least get started on writing one).

RKS’s picture

No problem. I actually didn't think you were referring to me and saying I was whining. I was just rebutting in general for the average person and not meaning to defend myself. But, you're right in wanting to move passed this and I can chalk it up to misunderstanding. I was pretty rude in my rant and while I haven't been having a bad day as far as I can tell, I must have just been in a mood or something. So I apologize for responding a bit harsher than what was really warranted.

Moving on, I will definitely look at reading through all of the posts and help with summarizing the issue.

xjm’s picture

I went to reroll the patch, but things have changed a lot since #180 so it's not a simple task. One question I ran into right away was where to put the admin form element for this setting in the new UI. I see two possibilities:

  1. On the content type's Edit tab at admin/structure/types/manage/typename, under the Display settings. That fieldset currently contains only the Display author and date information option.
  2. On the content type's Manage display tab under Teaser, in the form that appears when you click the gear doodad button. There's less space to work with there, but that's where the teaser trim length is.

I think #2 makes more sense, for a couple reasons. First, the location of the "read more" link is logically a part of the teaser display configuration and related to the trim length. Furthermore, this setting should be attached to any "Long text with summary" field, not just the default Body, and the setting should be configurable on a per-instance basis.

I also think we should change the element to a select box, especially if we use #2, because that is the UI pattern used for other elements there. (See the image styles settings for an example.)

Another question. It doesn't look like moshe's feedback about the theme function and rendering was addressed yet? From #170:

To clarify, this is a render() structure that I am envisoning at end of the patch. This assumes that that theme_read_more is renamed to theme_container() and adds an attributes param.

$build['read_more'] = array(
  '#link' => l(),
  '#theme' => 'container',
  '#attributes' => array('class' => array('read-more')),
)
return $build;
xjm’s picture

Issue summary: View changes

Added first summary

xjm’s picture

Issue summary: View changes

PEBCAK issue resulted in loss of my previous writeup, so saving this one in parts.

xjm’s picture

Issue summary: View changes

Summarizing the proposed solution.

xjm’s picture

Issue summary: View changes

Added points still under discussion.

xjm’s picture

Issue summary: View changes

And, finally, the actual status of the patch.

xjm’s picture

Summary added. I lost the first version due to a PEBCAK issue so it's a bit less detailed. :P

Before commenting on this issue, please review the summary above, especially the Remaining Tasks section. There are several points that still need to be decided, and the patch needs to be rewritten based on the current codebase and API. It's beyond a simple reroll.

xjm’s picture

Issue summary: View changes

Adding header IDs for links.

xjm’s picture

Issue summary: View changes

Missed parenthesis.

xjm’s picture

Retagging since we will need new screenshots and tests; sorry for the additional noise.

yoroy’s picture

Issue tags: +Needs design

I'm thinking this would benefit from a couple of sketches, wireframes that lay out the problem we're trying to solve here

Alan D.’s picture

Without reading this old thread, what was the use case for duplicating the read more link in content / links as mentioned in the issue summary?

I'm just cleaning up a contrib. module for 7.x-1.1 release and this handles the placement. The options that I came up independently were:

  $subform['placement'] = array(
    '#title' => t('Link placement'),
    '#options' => array(
      'none'   => t("Don't move (rendered in links element)"),
      'body_inline'  => t('Inline on body field'),
      'body_append'  => t('Append on new line after body field'),
      'text_inline'  => t('Inline on final long text field'),
      'text_append'  => t('Append on new line after final long text field'),
      'field_inline'  => t('Inline on new line after final field'),
      'field_append'  => t('Append on new line after final field'),
      'append'  => t('Append on new line after final field or addition field'),
    ),
  );

I'm inclined not to place the link in more than one location, unless others give a strong argument for this.

YesCT’s picture

Moved to 8.x in #193
This issue does not have an 8.x patch, but we could get a screenshot to show where the current link appears in 8.x

Add it to the issue summary in the ui changes section.
Contributor task for making screenshots for issues: http://drupal.org/node/1859584

YesCT’s picture

Issue summary: View changes

Punctuation fix.

xjm’s picture

Component: node.module » node system
Issue summary: View changes

(Merging "node system" and "node.module" components for 8.x; disregard.)

TallDavid’s picture

FileSize
52.2 KB

Screenshot showing placement of 'Read more' link in Drupal 8.0-alpha12:

'Read more' link in Drupal 8.0-alpha12

TallDavid’s picture

Issue summary: View changes

Updated Issue Summary to include a D8 before screenshot.

jhedstrom’s picture

Version: 8.0.x-dev » 8.1.x-dev

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.