mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-04-20 07:56:06 +02:00
Code Issues Projects Releases Wiki Activity

MDL-61378 forum: Standardize HTML output for posts

Browse Source 
Improve the format of the HTML representing a forum post.

1. Use <article> tag to mark it correctly.
2. Use <time> tag to include the real datetime information.
3. Use <address> tag to specify who and when the post came from.
4. Use heading level tags to mark the subject of each post.
5. Properly nest reply posts within their parent <article>
6. Use a header tab for each article to split the metadata from the content.
7. Use CSS to separate links, not text content.

Because posts are now nested properly, the heading for each one refers to it's own article
and we don't need to generate arbitrary heading level tags depending on how deep the post
is nested.
This commit is contained in:
Damyon Wiese 2018-09-10 15:52:07 +08:00
parent 129783b98c
commit 1bf3a76a59
5 changed files with 203 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
lib/moodlelib.php
View File

@ -2258,6 +2258,35 @@ function userdate($date, $format = '', $timezone = 99, $fixday = true, $fixhour
return $calendartype->timestamp_to_date_string($date, $format, $timezone, $fixday, $fixhour);
}
/**
* Returns a html "time" tag with both the exact user date with timezone information
* as a datetime attribute in the W3C format, and the user readable date and time as text.
*
* @package core
* @category time
* @param int $date the timestamp in UTC, as obtained from the database.
* @param string $format strftime format. You should probably get this using
* get_string('strftime...', 'langconfig');
* @param int|float|string $timezone by default, uses the user's time zone. if numeric and
* not 99 then daylight saving will not be added.
* {@link http://docs.moodle.org/dev/Time_API#Timezone}
* @param bool $fixday If true (default) then the leading zero from %d is removed.
* If false then the leading zero is maintained.
* @param bool $fixhour If true (default) then the leading zero from %I is removed.
* @return string the formatted date/time.
*/
function userdate_htmltime($date, $format = '', $timezone = 99, $fixday = true, $fixhour = true) {
$userdatestr = userdate($date, $format, $timezone, $fixday, $fixhour);
if (CLI_SCRIPT && !PHPUNIT_TEST) {
return $userdatestr;
}
$machinedate = new DateTime();
$machinedate->setTimestamp(intval($date));
$machinedate->setTimezone(core_date::get_user_timezone_object());
return html_writer::tag('time', $userdatestr, ['datetime' => $machinedate->format(DateTime::W3C)]);
}
/**
* Returns a formatted date ensuring it is UTF-8.
*

41
lib/tests/moodlelib_test.php
View File

@ -1714,73 +1714,85 @@ class core_moodlelib_testcase extends advanced_testcase {
'time' => '1309514400',
'usertimezone' => 'America/Moncton',
'timezone' => '0.0', // No dst offset.
'expectedoutput' => 'Friday, 1 July 2011, 10:00 AM'
'expectedoutput' => 'Friday, 1 July 2011, 10:00 AM',
'expectedoutputhtml' => '<time datetime="2011-07-01T07:00:00-03:00">Friday, 1 July 2011, 10:00 AM</time>'
),
array(
'time' => '1309514400',
'usertimezone' => 'America/Moncton',
'timezone' => '99', // Dst offset and timezone offset.
'expectedoutput' => 'Friday, 1 July 2011, 7:00 AM'
'expectedoutput' => 'Friday, 1 July 2011, 7:00 AM',
'expectedoutputhtml' => '<time datetime="2011-07-01T07:00:00-03:00">Friday, 1 July 2011, 7:00 AM</time>'
),
array(
'time' => '1309514400',
'usertimezone' => 'America/Moncton',
'timezone' => 'America/Moncton', // Dst offset and timezone offset.
'expectedoutput' => 'Friday, 1 July 2011, 7:00 AM'
'expectedoutput' => 'Friday, 1 July 2011, 7:00 AM',
'expectedoutputhtml' => '<time datetime="2011-07-01t07:00:00-03:00">Friday, 1 July 2011, 7:00 AM</time>'
),
array(
'time' => '1293876000 ',
'usertimezone' => 'America/Moncton',
'timezone' => '0.0', // No dst offset.
'expectedoutput' => 'Saturday, 1 January 2011, 10:00 AM'
'expectedoutput' => 'Saturday, 1 January 2011, 10:00 AM',
'expectedoutputhtml' => '<time datetime="2011-01-01T06:00:00-04:00">Saturday, 1 January 2011, 10:00 AM</time>'
),
array(
'time' => '1293876000 ',
'usertimezone' => 'America/Moncton',
'timezone' => '99', // No dst offset in jan, so just timezone offset.
'expectedoutput' => 'Saturday, 1 January 2011, 6:00 AM'
'expectedoutput' => 'Saturday, 1 January 2011, 6:00 AM',
'expectedoutputhtml' => '<time datetime="2011-01-01T06:00:00-04:00">Saturday, 1 January 2011, 6:00 AM</time>'
),
array(
'time' => '1293876000 ',
'usertimezone' => 'America/Moncton',
'timezone' => 'America/Moncton', // No dst offset in jan.
'expectedoutput' => 'Saturday, 1 January 2011, 6:00 AM'
'expectedoutput' => 'Saturday, 1 January 2011, 6:00 AM',
'expectedoutputhtml' => '<time datetime="2011-01-01T06:00:00-04:00">Saturday, 1 January 2011, 6:00 AM</time>'
),
array(
'time' => '1293876000 ',
'usertimezone' => '2',
'timezone' => '99', // Take user timezone.
'expectedoutput' => 'Saturday, 1 January 2011, 12:00 PM'
'expectedoutput' => 'Saturday, 1 January 2011, 12:00 PM',
'expectedoutputhtml' => '<time datetime="2011-01-01T12:00:00+02:00">Saturday, 1 January 2011, 12:00 PM</time>'
),
array(
'time' => '1293876000 ',
'usertimezone' => '-2',
'timezone' => '99', // Take user timezone.
'expectedoutput' => 'Saturday, 1 January 2011, 8:00 AM'
'expectedoutput' => 'Saturday, 1 January 2011, 8:00 AM',
'expectedoutputhtml' => '<time datetime="2011-01-01T08:00:00-02:00">Saturday, 1 January 2011, 8:00 AM</time>'
),
array(
'time' => '1293876000 ',
'usertimezone' => '-10',
'timezone' => '2', // Take this timezone.
'expectedoutput' => 'Saturday, 1 January 2011, 12:00 PM'
'expectedoutput' => 'Saturday, 1 January 2011, 12:00 PM',
'expectedoutputhtml' => '<time datetime="2011-01-01T00:00:00-10:00">Saturday, 1 January 2011, 12:00 PM</time>'
),
array(
'time' => '1293876000 ',
'usertimezone' => '-10',
'timezone' => '-2', // Take this timezone.
'expectedoutput' => 'Saturday, 1 January 2011, 8:00 AM'
'expectedoutput' => 'Saturday, 1 January 2011, 8:00 AM',
'expectedoutputhtml' => '<time datetime="2011-01-01T00:00:00-10:00">Saturday, 1 January 2011, 8:00 AM</time>'
),
array(
'time' => '1293876000 ',
'usertimezone' => '-10',
'timezone' => 'random/time', // This should show server time.
'expectedoutput' => 'Saturday, 1 January 2011, 6:00 PM'
'expectedoutput' => 'Saturday, 1 January 2011, 6:00 PM',
'expectedoutputhtml' => '<time datetime="2011-01-01T00:00:00-10:00">Saturday, 1 January 2011, 6:00 PM</time>'
),
array(
'time' => '1293876000 ',
'usertimezone' => '20', // Fallback to server time zone.
'timezone' => '99', // This should show user time.
'expectedoutput' => 'Saturday, 1 January 2011, 6:00 PM'
'expectedoutput' => 'Saturday, 1 January 2011, 6:00 PM',
'expectedoutputhtml' => '<time datetime="2011-01-01T18:00:00+08:00">Saturday, 1 January 2011, 6:00 PM</time>'
),
);
@ -1791,13 +1803,18 @@ class core_moodlelib_testcase extends advanced_testcase {
foreach ($testvalues as $vals) {
$USER->timezone = $vals['usertimezone'];
$actualoutput = userdate($vals['time'], '%A, %d %B %Y, %I:%M %p', $vals['timezone']);
$actualoutputhtml = userdate_htmltime($vals['time'], '%A, %d %B %Y, %I:%M %p', $vals['timezone']);
// On different systems case of AM PM changes so compare case insensitive.
$vals['expectedoutput'] = core_text::strtolower($vals['expectedoutput']);
$vals['expectedoutputhtml'] = core_text::strtolower($vals['expectedoutputhtml']);
$actualoutput = core_text::strtolower($actualoutput);
$actualoutputhtml = core_text::strtolower($actualoutputhtml);
$this->assertSame($vals['expectedoutput'], $actualoutput,
"Expected: {$vals['expectedoutput']} => Actual: {$actualoutput} \ndata: " . var_export($vals, true));
$this->assertSame($vals['expectedoutputhtml'], $actualoutputhtml,
"Expected: {$vals['expectedoutputhtml']} => Actual: {$actualoutputhtml} \ndata: " . var_export($vals, true));
}
}

170
mod/forum/lib.php
View File

@ -1321,7 +1321,9 @@ function forum_user_complete($course, $user, $mod, $forum) {
}
$discussion = $discussions[$post->discussion];
forum_print_post_start($post);
forum_print_post($post, $discussion, $forum, $cm, $course, false, false, false);
forum_print_post_end($post);
}
} else {
echo "<p>".get_string("noposts", "forum")."</p>";
@ -1629,7 +1631,7 @@ function forum_print_recent_activity($course, $viewfullnames, $timestart) {
$list .= html_writer::start_tag('li');
$list .= html_writer::start_div('head');
$list .= html_writer::div(userdate($post->modified, $strftimerecent), 'date');
$list .= html_writer::div(userdate_htmltime($post->modified, $strftimerecent), 'date');
if (!$authorhidden) {
$list .= html_writer::div(fullname($post, $viewfullnames), 'name');
}
@ -3099,6 +3101,94 @@ function forum_get_course_forum($courseid, $type) {
return $DB->get_record("forum", array("id" => "$forum->id"));
}
/**
* Return a static array of posts that are open.
*
* @return array
*/
function forum_post_nesting_cache() {
static $nesting = array();
return $nesting;
}
/**
* Return true for the first time this post was started
*
* @param int $id The id of the post to start
* @return bool
*/
function forum_should_start_post_nesting($id) {
$cache = forum_post_nesting_cache();
if (!array_key_exists($id, $cache)) {
$cache[$id] = 1;
return true;
} else {
$cache[$id]++;
return false;
}
}
/**
* Return true when all the opens are nested with a close.
*
* @param int $id The id of the post to end
* @return bool
*/
function forum_should_end_post_nesting($id) {
$cache = forum_post_nesting_cache();
if (!array_key_exists($id, $cache)) {
return true;
} else {
$cache[$id]--;
if ($cache[$id] == 0) {
unset($cache[$id]);
return true;
}
}
return false;
}
/**
* Start a forum post container
*
* @param object $post The post to print.
* @param bool $return Return the string or print it
* @return string
*/
function forum_print_post_start($post, $return = false) {
$output = '';
if (forum_should_start_post_nesting($post->id)) {
$output .= html_writer::start_tag('article');
$output .= html_writer::tag('a', '', array('id' => 'p'.$post->id));
}
if ($return) {
return $output;
}
echo $output;
return;
}
/**
* End a forum post container
*
* @param object $post The post to print.
* @param bool $return Return the string or print it
* @return string
*/
function forum_print_post_end($post, $return = false) {
$output = '';
if (forum_should_end_post_nesting($post->id)) {
$output .= html_writer::end_tag('article');
}
if ($return) {
return $output;
}
echo $output;
return;
}
/**
* Print a forum post
*
@ -3195,23 +3285,22 @@ function forum_print_post($post, $discussion, $forum, &$cm, $course, $ownpost=fa
echo $output;
return;
}
$output .= html_writer::tag('a', '', array('id'=>'p'.$post->id));
$output .= html_writer::start_tag('div', array('class'=>'forumpost clearfix',
'role' => 'region',
$output .= html_writer::start_tag('div', array('class' => 'forumpost clearfix',
'aria-label' => get_string('hiddenforumpost', 'forum')));
$output .= html_writer::start_tag('div', array('class'=>'row header'));
$output .= html_writer::tag('div', '', array('class'=>'left picture')); // Picture
$output .= html_writer::start_tag('header', array('class' => 'row header'));
$output .= html_writer::tag('div', '', array('class' => 'left picture', 'role' => 'presentation')); // Picture.
if ($post->parent) {
$output .= html_writer::start_tag('div', array('class'=>'topic'));
$output .= html_writer::start_tag('div', array('class' => 'topic'));
} else {
$output .= html_writer::start_tag('div', array('class'=>'topic starter'));
$output .= html_writer::start_tag('div', array('class' => 'topic starter'));
}
$output .= html_writer::tag('div', get_string('forumsubjecthidden','forum'), array('class' => 'subject',
'role' => 'header')); // Subject.
$output .= html_writer::tag('div', get_string('forumauthorhidden', 'forum'), array('class' => 'author',
'role' => 'header')); // Author.
$authorclasses = array('class' => 'author');
$output .= html_writer::tag('address', get_string('forumauthorhidden', 'forum'), $authorclasses); // Author.
$output .= html_writer::end_tag('div');
$output .= html_writer::end_tag('div'); // row
$output .= html_writer::end_tag('header'); // Header.
$output .= html_writer::start_tag('div', array('class'=>'row'));
$output .= html_writer::tag('div', '&nbsp;', array('class'=>'left side')); // Groups
$output .= html_writer::tag('div', get_string('forumbodyhidden','forum'), array('class'=>'content')); // Content
@ -3236,17 +3325,13 @@ function forum_print_post($post, $discussion, $forum, &$cm, $course, $ownpost=fa
echo $output;
return;
}
$output .= html_writer::tag('a', '', [
'id' => "p{$post->id}",
]);
$output .= html_writer::start_tag('div', [
'class' => 'forumpost clearfix',
'role' => 'region',
'aria-label' => get_string('forumbodydeleted', 'forum'),
]);
$output .= html_writer::start_tag('div', array('class' => 'row header'));
$output .= html_writer::tag('div', '', array('class' => 'left picture'));
$output .= html_writer::start_tag('header', array('class' => 'row header'));
$output .= html_writer::tag('div', '', array('class' => 'left picture', 'role' => 'presentation'));
$classes = ['topic'];
if (!empty($post->parent)) {
@ -3261,13 +3346,10 @@ function forum_print_post($post, $discussion, $forum, &$cm, $course, $ownpost=fa
]);
// Author.
$output .= html_writer::tag('div', '', [
'class' => 'author',
'role' => 'header',
]);
$output .= html_writer::tag('address', '', ['class' => 'author']);
$output .= html_writer::end_tag('div');
$output .= html_writer::end_tag('div'); // End row.
$output .= html_writer::end_tag('header'); // End header.
$output .= html_writer::start_tag('div', ['class' => 'row']);
$output .= html_writer::tag('div', '&nbsp;', ['class' => 'left side']); // Groups.
$output .= html_writer::tag('div', get_string('forumbodydeleted', 'forum'), ['class' => 'content']); // Content.
@ -3442,17 +3524,16 @@ function forum_print_post($post, $discussion, $forum, &$cm, $course, $ownpost=fa
$postbyuser->post = $post->subject;
$postbyuser->user = $postuser->fullname;
$discussionbyuser = get_string('postbyuser', 'forum', $postbyuser);
$output .= html_writer::tag('a', '', array('id'=>'p'.$post->id));
// Begin forum post.
$output .= html_writer::start_div('forumpost clearfix' . $forumpostclass . $topicclass,
['role' => 'region', 'aria-label' => $discussionbyuser]);
['aria-label' => $discussionbyuser]);
// Begin header row.
$output .= html_writer::start_div('row header clearfix');
$output .= html_writer::start_tag('header', ['class' => 'row header clearfix']);
// User picture.
if (!$authorhidden) {
$picture = $OUTPUT->user_picture($postuser, ['courseid' => $course->id]);
$output .= html_writer::div($picture, 'left picture');
$output .= html_writer::div($picture, 'left picture', ['role' => 'presentation']);
$topicclass = 'topic' . $topicclass;
}
@ -3462,26 +3543,25 @@ function forum_print_post($post, $discussion, $forum, &$cm, $course, $ownpost=fa
if (empty($post->subjectnoformat)) {
$postsubject = format_string($postsubject);
}
$output .= html_writer::div($postsubject, 'subject', ['role' => 'heading', 'aria-level' => '2']);
$output .= html_writer::div($postsubject, 'subject', ['role' => 'heading', 'aria-level' => '1']);
if ($authorhidden) {
$bytext = userdate($post->created);
$bytext = userdate_htmltime($post->created);
} else {
$by = new stdClass();
$by->date = userdate($post->created);
$by->date = userdate_htmltime($post->created);
$by->name = html_writer::link($postuser->profilelink, $postuser->fullname);
$bytext = get_string('bynameondate', 'forum', $by);
}
$bytextoptions = [
'role' => 'heading',
'aria-level' => '2',
'class' => 'author'
];
$output .= html_writer::div($bytext, 'author', $bytextoptions);
$output .= html_writer::tag('address', $bytext, $bytextoptions);
// End topic column.
$output .= html_writer::end_div();
// End header row.
$output .= html_writer::end_div();
$output .= html_writer::end_tag('header');
// Row with the forum post content.
$output .= html_writer::start_div('row maincontent clearfix');
@ -3538,7 +3618,7 @@ function forum_print_post($post, $discussion, $forum, &$cm, $course, $ownpost=fa
$output .= html_writer::end_tag('div'); // Content mask
$output .= html_writer::end_tag('div'); // Row
$output .= html_writer::start_tag('div', array('class'=>'row side'));
$output .= html_writer::start_tag('nav', array('class' => 'row side'));
$output .= html_writer::tag('div','&nbsp;', array('class'=>'left'));
$output .= html_writer::start_tag('div', array('class'=>'options clearfix'));
@ -3555,12 +3635,12 @@ function forum_print_post($post, $discussion, $forum, &$cm, $course, $ownpost=fa
$commandhtml = array();
foreach ($commands as $command) {
if (is_array($command)) {
$commandhtml[] = html_writer::link($command['url'], $command['text']);
$commandhtml[] = html_writer::link($command['url'], $command['text'], array('class' => 'nav-item nav-link'));
} else {
$commandhtml[] = $command;
}
}
$output .= html_writer::tag('div', implode(' | ', $commandhtml), array('class'=>'commands'));
$output .= html_writer::tag('div', implode(' ', $commandhtml), array('class' => 'commands nav'));
// Output link to post if required
if ($link) {
@ -3598,7 +3678,7 @@ function forum_print_post($post, $discussion, $forum, &$cm, $course, $ownpost=fa
// Close remaining open divs
$output .= html_writer::end_tag('div'); // content
$output .= html_writer::end_tag('div'); // row
$output .= html_writer::end_tag('nav'); // row
$output .= html_writer::end_tag('div'); // forumpost
// Mark the forum post as read if required
@ -3932,7 +4012,7 @@ function forum_print_discussion_header(&$post, $forum, $group = -1, $datestring
}
echo '<a href="'.$CFG->wwwroot.'/mod/forum/discuss.php?d='.$post->discussion.$parenturl.'">'.
userdate($usedate, $datestring).'</a>';
userdate_htmltime($usedate, $datestring).'</a>';
echo "</td>\n";
// is_guest should be used here as this also checks whether the user is a guest in the current course.
@ -5687,8 +5767,10 @@ function forum_print_latest_discussions($course, $forum, $maxdiscussions = -1, $
$discussion->forum = $forum->id;
forum_print_post_start($discussion);
forum_print_post($discussion, $discussion, $forum, $cm, $course, $ownpost, 0, $link, false,
'', null, true, $forumtracked);
forum_print_post_end($discussion);
break;
}
}
@ -5809,6 +5891,7 @@ function forum_print_discussion($course, $cm, $forum, $discussion, $post, $mode,
$postread = !empty($post->postread);
forum_print_post_start($post);
forum_print_post($post, $discussion, $forum, $cm, $course, $ownpost, $reply, false,
'', '', $postread, true, $forumtracked);
@ -5827,6 +5910,7 @@ function forum_print_discussion($course, $cm, $forum, $discussion, $post, $mode,
forum_print_posts_nested($course, $cm, $forum, $discussion, $post, $reply, $forumtracked, $posts);
break;
}
forum_print_post_end($post);
}
@ -5859,8 +5943,10 @@ function forum_print_posts_flat($course, &$cm, $forum, $discussion, $post, $mode
$postread = !empty($post->postread);
forum_print_post_start($post);
forum_print_post($post, $discussion, $forum, $cm, $course, $ownpost, $reply, $link,
'', '', $postread, true, $forumtracked);
forum_print_post_end($post);
}
}
@ -5892,8 +5978,10 @@ function forum_print_posts_threaded($course, &$cm, $forum, $discussion, $parent,
$postread = !empty($post->postread);
forum_print_post_start($post);
forum_print_post($post, $discussion, $forum, $cm, $course, $ownpost, $reply, $link,
'', '', $postread, true, $forumtracked);
forum_print_post_end($post);
} else {
if (!forum_user_can_see_post($forum, $discussion, $post, null, $cm, true)) {
if (forum_user_can_see_post($forum, $discussion, $post, null, $cm, false)) {
@ -5908,7 +5996,7 @@ function forum_print_posts_threaded($course, &$cm, $forum, $discussion, $parent,
} else {
$by = new stdClass();
$by->name = fullname($post, $canviewfullnames);
$by->date = userdate($post->modified);
$by->date = userdate_htmltime($post->modified);
$byline = ' ' . get_string("bynameondate", "forum", $by);
$subject = format_string($post->subject, true);
}
@ -5965,9 +6053,11 @@ function forum_print_posts_nested($course, &$cm, $forum, $discussion, $parent, $
$post->subject = format_string($post->subject);
$postread = !empty($post->postread);
forum_print_post_start($post);
forum_print_post($post, $discussion, $forum, $cm, $course, $ownpost, $reply, $link,
'', '', $postread, true, $forumtracked);
forum_print_posts_nested($course, $cm, $forum, $discussion, $post, $reply, $forumtracked, $posts);
forum_print_post_end($post);
echo "</div>\n";
}
}
@ -6154,7 +6244,7 @@ function forum_print_recent_mod_activity($activity, $courseid, $detail, $modname
$output .= html_writer::link($discussionurl, $content->subject);
$output .= html_writer::end_div();
$timestamp = userdate($activity->timestamp);
$timestamp = userdate_htmltime($activity->timestamp);
if ($authorhidden) {
$authornamedate = $timestamp;
} else {

12
mod/forum/styles.css
View File

@ -315,3 +315,15 @@ span.unread {
#page-mod-forum-view img.timedpost {
margin-right: 5px;
}
.path-mod-forum article .nav .nav-link:first-of-type {
margin-left: auto;
}
.path-mod-forum.dir-rtl article .nav .nav-link:first-of-type {
margin-left: 0;
margin-right: auto;
}
.path-mod-forum article .nav .nav-link + .nav-link {
border-left: 1px solid #ddd;
}

6
mod/forum/tests/behat/posts_ordering_blog.feature
View File

@ -72,9 +72,9 @@ Feature: Blog posts are always displayed in reverse chronological order
#
# Make sure the order of the blog posts is still reverse chronological.
#
Then I should see "This is the third post" in the "//div[contains(concat(' ', normalize-space(@class), ' '), ' forumpost ')][position()=1]" "xpath_element"
And I should see "This is the second post" in the "//div[contains(concat(' ', normalize-space(@class), ' '), ' forumpost ')][position()=2]" "xpath_element"
And I should see "This is the first post" in the "//div[contains(concat(' ', normalize-space(@class), ' '), ' forumpost ')][position()=3]" "xpath_element"
Then I should see "This is the third post" in the "//article[position()=1]" "xpath_element"
And I should see "This is the second post" in the "//article[position()=2]" "xpath_element"
And I should see "This is the first post" in the "//article[position()=3]" "xpath_element"
#
# Make sure the next/prev navigation uses the same order of the posts.
#