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:
- http://drupal.org/project/read_more (Drupal 7)
- http://drupal.org/project/ed_readmore (Drupal 6 and earlier)
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:
- Update the patch to use the field API, and provide the configuration option for any Long text with summary fields.
- Update the theme function to use the render API (see #170).
- Incorporate final decisions for the points above.
User interface changes
- Current appearance of '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.)
Comment | File | Size | Author |
---|---|---|---|
#210 | ReadMe-Drupal8-600h.png | 52.2 KB | TallDavid |
#180 | 16161_readmore_7.diff | 14.7 KB | jeffschuler |
#169 | read_more_6_3.patch | 14.61 KB | jeffschuler |
#161 | read_more_6_2.patch | 14.4 KB | jeffschuler |
#140 | read_more_5.patch | 12.74 KB | mr.baileys |
Comments
Comment #1
factoryjoe CreditAttribution: factoryjoe commented+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.
Comment #2
moshe weitzman CreditAttribution: moshe weitzman commentedif you prefer, this could be done with nodeapi('view') in a module instead opf in the theme.
Comment #3
chrisada CreditAttribution: chrisada commentedBut 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?
Comment #4
factoryjoe CreditAttribution: factoryjoe commentedThe 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").
Comment #5
David Hull CreditAttribution: David Hull commentedI 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:
The preg_replace is there to attempt to move the "[...]" inside the last paragraph.
Comment #6
chrisada CreditAttribution: chrisada commentedUpdating applicable version to see if there is still interests in this.
Comment #7
Gurpartap Singh CreditAttribution: Gurpartap Singh commentedThe paging.module(5.x release) has example of overriding "Read more" link. http://drupal.org/project/paging
Comment #8
Gurpartap Singh CreditAttribution: Gurpartap Singh commentedThe ed_readmore.module gives you this feature. But this is on feature wish list for 6.x too.
Comment #9
PanchoThough 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.
Comment #10
SeanBannister CreditAttribution: SeanBannister commentedHey 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.
Comment #11
mcurry CreditAttribution: mcurry commented*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?
Comment #12
mcurry CreditAttribution: mcurry commentedSome goals or features I'd like to suggest:
Comment #13
SeanBannister CreditAttribution: SeanBannister commentedHey 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.
Comment #14
psicomante CreditAttribution: psicomante commented+1 to this feature!
Comment #15
maartenvg CreditAttribution: maartenvg commented@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.
Comment #16
maartenvg CreditAttribution: maartenvg commentedI'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.
Comment #17
psicomante CreditAttribution: psicomante commentedi 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.
Comment #18
SeanBannister CreditAttribution: SeanBannister commentedWill test patch soon,
I agree there's no need for a GUI. Moving the Read More link just makes sense.
Comment #19
thomas23@drupal.org CreditAttribution: thomas23@drupal.org commentedsubscribe and +1 to get it out of node->links
Comment #20
maartenvg CreditAttribution: maartenvg commented#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.
Comment #21
Dave ReidPatch 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(' ', ' ', 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  , 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.
Comment #22
maartenvg CreditAttribution: maartenvg commentedad 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.
Comment #23
Dave ReidNew 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.
Comment #24
maartenvg CreditAttribution: maartenvg commentedLooks 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.
Comment #25
maartenvg CreditAttribution: maartenvg commentedHmm. 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 tagEnding in
...</h1-6>
: append after end tag (on next line)Ending in
...</p></blockquote>
: append before end tagEnding in
...</blockquote>
: append before end tagEnding 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>
: beforeEnding in
</p></div>
: beforeEnding in
</pre>
: afterEnding in
</address>
: before (will alter the style of the tag, but probably not too much)How to handle lists, tables and forms?
Comment #26
SeanBannister CreditAttribution: SeanBannister commentedJust 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.
Comment #27
maartenvg CreditAttribution: maartenvg commentedThe problem is that a teaser not always ends neatly with a
This is a heading Read more</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 thisNot 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).
Comment #28
SeanBannister CreditAttribution: SeanBannister commentedOk, 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?
Comment #29
mcurry CreditAttribution: mcurry commentedFinding 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?
Comment #30
dugh CreditAttribution: dugh commentedI 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.
Comment #31
ericjam CreditAttribution: ericjam commentedhow do i implement this in 6x? how do i do this patch thingie?
Comment #32
rfayPlease do add this feature! It's an excellent one and should be in core.
Comment #33
SeanBannister CreditAttribution: SeanBannister commentedComment #34
SeanBannister CreditAttribution: SeanBannister commentedComment #35
bsherwood CreditAttribution: bsherwood commented+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.
Comment #36
geerlingguy CreditAttribution: geerlingguy commentedSimply 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.
Comment #37
dddave CreditAttribution: dddave commentedSubscribing!
Comment #38
momper CreditAttribution: momper commented+1
Comment #39
mean0dspt CreditAttribution: mean0dspt commented+1
Comment #40
3duardo CreditAttribution: 3duardo commented+1
Comment #41
jgossage CreditAttribution: jgossage commented+1
when the end tag issues are resolved
Comment #42
Todd Nienkerk CreditAttribution: Todd Nienkerk commented+1. Excellent!
Comment #43
_nolocation CreditAttribution: _nolocation commented+1
Comment #44
Todd Nienkerk CreditAttribution: Todd Nienkerk commentedPlease note that Read More is again under active development for inclusion in the Drupal.org upgrade and retheming. Changes are afoot!
Comment #45
funana CreditAttribution: funana commented+ 1
Comment #46
Todd Nienkerk CreditAttribution: Todd Nienkerk commentedRegarding 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.Comment #47
David_Rothstein CreditAttribution: David_Rothstein commentedThis 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 :)
Comment #48
jackalope CreditAttribution: jackalope commented+1
Comment #49
Gurpartap Singh CreditAttribution: Gurpartap Singh commentedToo many +++s there. I counted, they are 13. Someone should now fold their sleeves and sit with a cup of coffee.
Comment #50
Anonymous (not verified) CreditAttribution: Anonymous commented*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.
Comment #51
Todd Nienkerk CreditAttribution: Todd Nienkerk commented@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.
Comment #52
David_Rothstein CreditAttribution: David_Rothstein commentedThat'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?
Comment #53
Dave ReidYes, this should be left separately since it applies to getting the module functionality into Drupal core.
Comment #54
Todd Nienkerk CreditAttribution: Todd Nienkerk commentedDavid_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.
Comment #55
babbage CreditAttribution: babbage commented+1. (Still no coffee.)
Comment #56
Cool_Goose CreditAttribution: Cool_Goose commented+
Comment #57
arnd CreditAttribution: arnd commentedI know this issue also from another cms and several users have complained about that.
Comment #58
bendshead CreditAttribution: bendshead commented+1 yup
Comment #59
zzlong CreditAttribution: zzlong commentedI support this request, kind of must-have enhancement for Drupal.
Comment #60
jbc CreditAttribution: jbc commented+1
Comment #61
saccard CreditAttribution: saccard commentedyes, should be a Drupal7 feature
Comment #62
degure CreditAttribution: degure commentedI also would appreciate more flexibility for the "read more" link. Thanks for considering it.
Comment #63
MGParisi CreditAttribution: MGParisi commentedYes! 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.
Comment #64
Aveu CreditAttribution: Aveu commentedI 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.
Comment #65
reglogge CreditAttribution: reglogge commentedall for it
+1
Comment #66
Duplika CreditAttribution: Duplika commented+1
Comment #67
axbom CreditAttribution: axbom commented+1 this is always one of the first things I have to fix on a new install
Comment #68
mkrakowiak CreditAttribution: mkrakowiak commentedsubscribing
Comment #69
Todd Nienkerk CreditAttribution: Todd Nienkerk commentedIn 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.
Comment #70
Gerald Mengisen CreditAttribution: Gerald Mengisen commented+1
Comment #71
danfinney CreditAttribution: danfinney commented+2 (my wife thinks so too)
Comment #72
Damien Tournoud CreditAttribution: Damien Tournoud commentedPlease avoid all kind of string parsing. That's ugly and fragile. It should be easy, fast and robust to do that using DOM manipulation.
Comment #73
kevcol CreditAttribution: kevcol commentedI'd like to see this in Drupal 7. Is much more intuitive and far easier than workarounds.
Comment #74
noovot CreditAttribution: noovot commented+1 definitely
Comment #75
morleti CreditAttribution: morleti commented+1! absolutely yes! and great thanks to developers who implement it!
Comment #76
yraffah CreditAttribution: yraffah commented+1
Comment #77
jorsol CreditAttribution: jorsol commentedIt must be a feature in Drupal 7...
+1
Comment #78
zooki CreditAttribution: zooki commentedTHANK YOU +1
Comment #79
iamwhoiam CreditAttribution: iamwhoiam commentedDon't know why its not already in!
Comment #80
webchickBecause people keep posting totally unhelpful things like "+1" and "Don't know why its not already in" rather than actually working on the patch.
Comment #81
geerlingguy CreditAttribution: geerlingguy commented@ 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.
Comment #82
cazador CreditAttribution: cazador commentedShould be in core
Comment #83
superfly CreditAttribution: superfly commented+1 should be in core. this is one of the modules I *always* install.
Comment #84
webchickOK. 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.
Comment #85
Gurpartap Singh CreditAttribution: Gurpartap Singh commentedwebchick, +1.
Comment #86
MGParisi CreditAttribution: MGParisi commented@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.
Comment #87
MGParisi CreditAttribution: MGParisi commentedSorry, forgot to add the following XML tags
<sarcasm>first paragraph</sarcasm>
<unusualsolution>Second Paragraph </unusualsolution>
Note Proper use of XML is important :-p
Comment #88
Rob C CreditAttribution: Rob C commented+1 this has to be in core.
Comment #89
geerlingguy CreditAttribution: geerlingguy commentedI 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.
Comment #90
MGParisi CreditAttribution: MGParisi commentedEdited beyond original post.
Comment #91
MGParisi CreditAttribution: MGParisi commentedThere 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.
Comment #92
ericjam CreditAttribution: ericjam commentedNot to bust balls but the ed_readmore module has saved my life. Maybe you could take a page from them.
Comment #93
yoroy CreditAttribution: yoroy commentedre: #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.
Comment #95
espirates CreditAttribution: espirates commented+2
Comment #96
Damien Tournoud CreditAttribution: Damien Tournoud commentedThe 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.
Comment #97
Aveu CreditAttribution: Aveu commentedMGParisi: 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
Comment #98
webchickNo, 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.
Comment #99
webchickOh. 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.
Comment #100
dugh CreditAttribution: dugh commentedThe code's in the ed_readmore module, which solves this issue
Comment #101
kaakuu CreditAttribution: kaakuu commentedMy 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.
Comment #102
geerlingguy CreditAttribution: geerlingguy commentedOkay, 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!
Comment #103
MGParisi CreditAttribution: MGParisi commentedGeerlingguy, 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!
Comment #104
MGParisi CreditAttribution: MGParisi commentedAlso, 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!
Comment #105
geerlingguy CreditAttribution: geerlingguy commentedThe 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 :-)
Comment #106
Augusto Ellacuriaga CreditAttribution: Augusto Ellacuriaga commented+1 must be in the core
Comment #107
mr.baileysWell, I wanted to add a +1 to this thread, but didn't dare doing so without at least some contribution, so here goes:
"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:
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...
I'm not sure if this wouldn't crowd the interface too much.
Setting to CNR so the testbot can take a look...
Comment #109
mr.baileysFixed an error in node_link() that caused this patch to fail.
Comment #110
geerlingguy CreditAttribution: geerlingguy commentedI 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.
Comment #111
babbage CreditAttribution: babbage commentedInstallation
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
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
Well done for actually writing a patch on this ancient issue! Now let's get it in D7 at last... :)
Comment #112
mr.baileys@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 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.
Comment #113
kaakuu CreditAttribution: kaakuu commentedIs this http://drupal.org/files/issues/read_more_0.png exactly going to core ? If so Cheers!!
Comment #114
geerlingguy CreditAttribution: geerlingguy commented@ 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.
Comment #115
webchickWow! 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.
Comment #116
SeanBannister CreditAttribution: SeanBannister commentedJust 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.
Comment #117
babbage CreditAttribution: babbage commented@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?
Comment #118
mr.baileysI'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?
Comment #119
SeanBannister CreditAttribution: SeanBannister commentedI 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.Comment #120
SeanBannister CreditAttribution: SeanBannister commentedJust 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()
RequiredThis 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 postI would propose we simplify it a little :
Read more
()
Disabled()
Display in node summary()
Display in links sectionI'd appreciate some feedback on this.
Comment #121
moshe weitzman CreditAttribution: moshe weitzman commentedGood lord. That DOMDocument code in test.module is a bit much for core IMO. Would be good to get some opinions on that.
Comment #122
mr.baileys@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
Comment #123
Damien Tournoud CreditAttribution: Damien Tournoud commentedYay for proper DOM manipulation. This looks really awesome, mr.baileys.
Comment #125
babbage CreditAttribution: babbage commentedRather 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.)
Comment #126
yched CreditAttribution: yched commented(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...
Comment #127
Damien Tournoud CreditAttribution: Damien Tournoud commentedYves, 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.
Comment #128
SeanBannister CreditAttribution: SeanBannister commented@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.
Comment #129
yched CreditAttribution: yched commented@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'
Comment #130
Gurpartap Singh CreditAttribution: Gurpartap Singh commented+1
Comment #131
babbage CreditAttribution: babbage commentedSo... 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...
Comment #132
mr.baileysSetting this to NR for the testbot.
@dbabbage:
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.
Comment #133
yoroy CreditAttribution: yoroy commented- 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!
Comment #134
yoroy CreditAttribution: yoroy commentedstatus…
Comment #135
mr.baileys@yoroy, "append to summary" would certainly suffice, new patch attached. Also included an up-to-date screenshot of the UI.
Comment #136
SeanBannister CreditAttribution: SeanBannister commentedTested the patch and couldn't break it.
Comment #137
SeanBannister CreditAttribution: SeanBannister commentedWhen 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???
Comment #138
babbage CreditAttribution: babbage commentedRe #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'. :)
Comment #139
yoroy CreditAttribution: yoroy commenteddbabbage: all good then :-)
Comment #140
mr.baileysAdded tests...
Comment #141
mr.baileysgentle nudge... Would be great to get this 4 1/2 year old issue resolved...
Comment #142
SeanBannister CreditAttribution: SeanBannister commentedTested #140 patch, works great.
Comment #143
jeffschulerTested #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...
Comment #144
moshe weitzman CreditAttribution: moshe weitzman commentedIs 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().
I think this should be named theme_container or somesuch. It should take an $attributes array and 'class' => 'read-more' should be passed in.
Comment #145
mlncn CreditAttribution: mlncn commentedHuge 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:
Comment #146
moshe weitzman CreditAttribution: moshe weitzman commentedi asked a couple questions right before Ben.
Comment #147
mlncn CreditAttribution: mlncn commented(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!)
Comment #148
mlncn CreditAttribution: mlncn commentedCross-posted again. I quit.
Comment #149
SeanBannister CreditAttribution: SeanBannister commentedI 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".
Comment #150
babbage CreditAttribution: babbage commentedI 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?
Comment #151
yoroy CreditAttribution: yoroy commentedOn
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.
Comment #152
Bojhan CreditAttribution: Bojhan commentedLets work a bit on the Append.. that sounds rather programming-lingo to me.
Comment #153
ctmattice1 CreditAttribution: ctmattice1 commentedHow 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"
Comment #154
SeanBannister CreditAttribution: SeanBannister commented@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.
Comment #155
Bojhan CreditAttribution: Bojhan commentedThats 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.
Comment #156
SeanBannister CreditAttribution: SeanBannister commentedI 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.
Comment #157
yched CreditAttribution: yched commented+ 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.
Comment #158
dddave CreditAttribution: dddave commented+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.
Comment #159
jeffschuler+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.
Comment #160
amc CreditAttribution: amc commentedWait 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?
Comment #161
jeffschuler@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) :
(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.
Comment #162
mcrittenden CreditAttribution: mcrittenden commentedSubscribe.
Comment #163
yoroy CreditAttribution: yoroy commentedre: 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
Comment #165
Bojhan CreditAttribution: Bojhan commentedI am fine with fixing the text post code freeze.
Comment #166
babbage CreditAttribution: babbage commentedyoroy: 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"?)
Comment #167
geerlingguy CreditAttribution: geerlingguy commented+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.
Comment #168
yoroy CreditAttribution: yoroy commentedBut let's not hold this up on wording… Append it is then. Somebody up for rerolling?
Comment #169
jeffschulerRe-rolled #161 with a working
node.test
.Comment #170
moshe weitzman CreditAttribution: moshe weitzman commentedTo 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.
Comment #172
yoroy CreditAttribution: yoroy commentedComment #173
yoroy CreditAttribution: yoroy commentedFunctionally seems to have been ok. slightly abusing the rtbc status to see if we can get one last glance at this before freeze?
Comment #174
SeanBannister CreditAttribution: SeanBannister commentedWe 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.
Comment #176
Cliff CreditAttribution: Cliff commentedSubscribe
Comment #177
yoroy CreditAttribution: yoroy commentedSure bot?
Comment #178
mean0dspt CreditAttribution: mean0dspt commentedthere's a code freeze coming. Don't you think it's time to commit this patch already?
Comment #179
yoroy CreditAttribution: yoroy commentedProbably needs a reroll
Comment #180
jeffschulerRe-rolled to chase HEAD.
Comment #182
Cliff CreditAttribution: Cliff commentedI'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:
Comment #183
yoroy CreditAttribution: yoroy commentedThis 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.
Comment #184
Cliff CreditAttribution: Cliff commented@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.
Comment #185
babbage CreditAttribution: babbage commentedI 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.
Comment #186
Cliff CreditAttribution: Cliff commentedThose are good revisions, @dbabbage. I like it when someone takes an idea of mine and improves upon it!
Comment #187
Cliff CreditAttribution: Cliff commented[How did that post twice?]
Comment #188
Everett Zufelt CreditAttribution: Everett Zufelt commented+1 for radios instead of checkboxes. And also +1 for allowing users to modify text for link, even if only through the theming function.
Comment #189
Aveu CreditAttribution: Aveu commentedThis 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
Comment #193
yoroy CreditAttribution: yoroy commentedBummer :\
Comment #194
femrich CreditAttribution: femrich commentedput off for drupal 8. unbelievable.
Comment #195
Dave Reid@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
Comment #196
femrich CreditAttribution: femrich commented@ 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...
Comment #197
RKS CreditAttribution: RKS commentedI 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.
Comment #198
David_Rothstein CreditAttribution: David_Rothstein commentedRKS: 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.)
Comment #199
RKS CreditAttribution: RKS commentedOkay, 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.
Comment #200
mcurry CreditAttribution: mcurry commentedUN-subscribe :D
Comment #201
David_Rothstein CreditAttribution: David_Rothstein commented@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).
Comment #202
RKS CreditAttribution: RKS commentedNo 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.
Comment #203
xjmI 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:
admin/structure/types/manage/typename
, under the Display settings. That fieldset currently contains only the Display author and date information option.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:
Comment #203.0
xjmAdded first summary
Comment #203.1
xjmPEBCAK issue resulted in loss of my previous writeup, so saving this one in parts.
Comment #203.2
xjmSummarizing the proposed solution.
Comment #203.3
xjmAdded points still under discussion.
Comment #203.4
xjmAnd, finally, the actual status of the patch.
Comment #204
xjmSummary 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.
Comment #204.0
xjmAdding header IDs for links.
Comment #204.1
xjmMissed parenthesis.
Comment #205
xjmRetagging since we will need new screenshots and tests; sorry for the additional noise.
Comment #206
yoroy CreditAttribution: yoroy commentedI'm thinking this would benefit from a couple of sketches, wireframes that lay out the problem we're trying to solve here
Comment #207
Alan D. CreditAttribution: Alan D. commentedWithout 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:
I'm inclined not to place the link in more than one location, unless others give a strong argument for this.
Comment #208
YesCT CreditAttribution: YesCT commentedMoved 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
Comment #208.0
YesCT CreditAttribution: YesCT commentedPunctuation fix.
Comment #209
xjm(Merging "node system" and "node.module" components for 8.x; disregard.)
Comment #210
TallDavid CreditAttribution: TallDavid commentedScreenshot showing placement of 'Read more' link in Drupal 8.0-alpha12:
Comment #211
TallDavid CreditAttribution: TallDavid commentedUpdated Issue Summary to include a D8 before screenshot.
Comment #212
jhedstrom