[4.x] More flexible display of relative dates. by Harmageddon · Pull Request #33023 · joomla/joomla-cms

Harmageddon · 2021-04-05T17:27:10Z

Summary of Changes

This is an alternative solution to #31675 and fixes the concerns discussed in #32911 (comment) and the following comments.
This PR introduces a new HTMLHelper method called date.relativeFormatted which can be used to display dates in a relative format without losing important information.

Key features:

Relative or absolute format is chosen based on user preference.
Using the flag $forceRelative, relative display can be forced, ignoring the user's preference (use carefully!).
In addition to the relative date display, the full absolute date is shown either in a tooltip or as small text below (like it is currently in the actionlogs / privacy components). This can be deactivated via method parameter.
If we use the same format for an absolute display as for the full date (DATE_FORMAT_LC6), it is not displayed twice.

I also added unit tests for the new method.

Testing Instructions

Log in to backend.
In the user menu (top right), click "Edit Account". In the Tab "Basic Settings", you should find a new parameter "Show Relative Dates in Lists" that defaults to true.
Check the following views with this parameter once enabled and once disabled:
- Module "Latest Actions" on the administrator home dashboard.
- Module "Logged-in users" on the administrator home dashboard (attention: not yet implemented, only after [4] Show relative date instead of date time in logged in module #32911 is merged and I updated this PR here accordingly)
- Users - User Actions Log
- Users - Privacy - Requests: You have to create a new request here if there are none yet.
- Users - Privacy - Consents: You have to give consent with at least one user beforehand. To do this, enable the plugin "System - Privacy Consent" and log in to frontend with a user account.

Actual result BEFORE applying this Pull Request

The dates in the mentioned components and modules are relative for dates which are more recent than a month. For user actions log and the privacy views, the full date is displayed below the relative date.

For the dashboard modules, the full date is not shown anywhere.

Expected result AFTER applying this Pull Request

With "Show Relative Dates in Lists" enabled

In the dashboard modules, the relative dates have a tooltip now showing the full exact date of the action.

In the actions log and privacy views, the display is the same as before.

With "Show Relative Dates in Lists" disabled

Dates in the mentioned locations are displayed in an absolute format. The full date and time is shown below or in the tooltip, respectively.

Documentation Changes Required

Maybe we need to document this user parameter somewhere?

Open Discussion Points

Should we include a default setting for users who have not set this parameter by themselves? Or just default to relative display (the way this PR does it now).
If date display is set to relative, the date format is DATE_FORMAT_LC6, and the date is more than one month in the past, we get the same date twice. The full date tooltip or div does not provide any additional information in this case. Should we try to detect this case or just leave it that way?
Which date format to use for the actions log / privacy components? Currently, it defaults to DATE_FORMAT_LC1, which is "Monday, 05 April 2021". In these list formats, something like DATE_FORMAT_LC5 (2021-05-04 19:00) or DATE_FORMAT_LC6 (2021-05-04 19:00:00) might be better?
Is there a chance to get this in 4.0 or should I directly re-target it to 4.1? In my view, [4] Display relative date instead of date stamp in user action log module #32910 and [4] Show relative date instead of date time in logged in module #32911 (one already merged into 4.0, one RTC) take away some information from these modules. This PR would "fix" this. But I also see that this might count as a new feature, so 4.1 would be okay as well I guess.

administrator/components/com_users/forms/user.xml

brianteeman · 2021-04-06T13:33:05Z

Please dont use bootstrap tooltips for this - they are horrible for accessibility.

Please look at the way the code is done for the search box and use the pseudo tip code for that

brianteeman · 2021-04-06T14:17:27Z

BTW I think we used to have this functionality and it was removed

brianteeman · 2021-04-06T14:21:18Z

Although I do agree a RELATIVE date should have a popover with the full timestamp - that makes sense.

I think if my memory is correct the problem was that the relative date was only for the first month and after that you got the real date and then the tooltip was a duplicate. But my memory might be tricking me

ChristineWk · 2021-04-06T16:13:20Z

Although I do agree a RELATIVE date should have a popover with the full timestamp - that makes sense.

Do you mean in this way: Patch: ON, View enabled:

sorry, hv trbls to add screenshot ....

Co-authored-by: Brian Teeman <brian@teeman.net>

Harmageddon · 2021-04-06T17:08:58Z

Please dont use bootstrap tooltips for this - they are horrible for accessibility.

Please look at the way the code is done for the search box and use the pseudo tip code for that

Done, thank you!

Is this screenshot wrong? Whats the point of a tool tip that shows the same as the text hovered over?

That's a good point. Technically, it's not the same (in case of absolute formatting and the same date format, the tooltip would not be shown), but it's very close. I added the tooltip for situations where we use a less precise format like the second one you showed here: #33023 (comment)

I think if my memory is correct the problem was that the relative date was only for the first month and after that you got the real date and then the tooltip was a duplicate. But my memory might be tricking me

I don't know if that was the reason back then, but it is an issue, yes (see discussion point 2 in the description). I could try to detect and avoid this case, but then we would have an inconsistency between "two days ago (tooltip: 2021-04-04 19:05:00)" and "2021-02-02 19:00:00 (without tooltip)".

Not to kill contributions, but personally, as I have already commented elsewhere:

The home dashboard is an overview of the RECENT happenings on a site therefore it MAKES SENSE that this screen should have relative dates.

On normal listing screens date stamps are better, and can be ordered etc,

And as you have said... "do we really need another option"....

As discussed in #31675, we do need another option, at least for the list case, because different users have different preferences there. Why not use one option for all instead of adding such an option everywhere?

ChristineWk · 2021-04-06T18:04:30Z

Although I do agree a RELATIVE date should have a popover with the full timestamp - that makes sense.

Do you mean in this way: Patch: ON, View enabled:

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33023.}

ChristineWk · 2021-04-06T18:17:33Z

The other point:

I think if my memory is correct the problem was that the relative date was only for the first month and after that you got the real date and then the tooltip was a duplicate. But my memory might be tricking me

@ Harmageddon wrote: I don't know if that was the reason back then, but it is an issue, yes (see discussion point 2 in the description). I could try to detect and avoid this case, but then we would have an inconsistency between "two days ago (tooltip: 2021-04-04 19:05:00)" and "2021-02-02 19:00:00 (without tooltip)".

sorry, I can't send screenshot via Github, therefore here:

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33023.}

Quy · 2021-04-06T19:03:19Z

libraries/src/HTML/Helpers/Date.php

+		$date, $unit = null, $time = null, $format = null, $forceRelative = false, $showAbsoluteDate = 'tooltip'
+	)


Suggested change

$date, $unit = null, $time = null, $format = null, $forceRelative = false, $showAbsoluteDate = 'tooltip'

)

$date, $unit = null, $time = null, $format = null, $forceRelative = false, $showAbsoluteDate = 'tooltip')

@Quy Hmm, PHPCS has a different opinion about that: https://ci.joomla.org/joomla/joomla-cms/41800/1/6 What is right here?

Per Joomla! Coding Standards:

If a function definition goes over multiple lines, all lines must be indented with one tab and the closing bracket must go on the same line as the last parameter.

dgrammatiko · 2021-04-07T20:36:16Z

libraries/src/HTML/Helpers/Date.php

+
+		if ($showAbsoluteDate === 'below')
+		{
+			return $dateMain . '<div class="small">' . $dateAbsolute . '</div>';


Use <date>

Is this a custom element? Can't find it anywhere in core or the HTML5 reference. Or are you referring to <time>?

Yes: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/time
Also FYI there's a <small> element also so div and span should be really avoided.

The point of using the proper element is that this text doesn't convey any message for disabled people but time does.

Better like this?

dgrammatiko · 2021-04-07T20:36:36Z

libraries/src/HTML/Helpers/Date.php

+
+		if ($showAbsoluteDate === 'tooltip')
+		{
+			return '<span>' . $dateMain . '</span><div role="tooltip">' . $dateAbsolute . '</div>';


DO NOT use tooltip...

Do you have an alternative suggestion? Always displaying the full date below the relative one might lead to crowded layouts in some cases, and confuse rather than help users. And not displaying this additional information at all seem worse to me than a tooltip.

dgrammatiko · 2021-04-07T20:40:34Z

libraries/src/HTML/Helpers/Date.php

+	 * @since   __DEPLOY_VERSION__
+	 */
+	public static function relativeFormatted(
+		$date, $unit = null, $time = null, $format = null, $forceRelative = false, $showAbsoluteDate = 'tooltip'


Any function with more than 4 arguments is realistically unusable (nobody will ever remember 6 params in a row, never).
Also if you want to have Bootstrap mixed here I would suggest creating a JLayout instead of an HTMLHelper. HTMLHelpers need a plugin for an override, JLayouts not and because not everybody is happy with BS easy overriding is better for the future of the project

Dude, it's not about IDE's it's about WRONG patterns, according to everybody in the business before we were even born (ref: https://softwareengineering.stackexchange.com/a/145073 )

Any function with more than 4 arguments is realistically unusable (nobody will ever remember 6 params in a row, never).

True. But my goal was to create a rather generic function that can be used in various places throughout the core for different displays of dates. I could rearrange the parameters, so the (expectedly) less used parameters can be omitted and use default values.

Also if you want to have Bootstrap mixed here I would suggest creating a JLayout instead of an HTMLHelper. HTMLHelpers need a plugin for an override, JLayouts not and because not everybody is happy with BS easy overriding is better for the future of the project

Following the suggestion of Brian above, there is no Bootstrap anymore in this PR.

BertaOctech · 2021-04-08T17:30:36Z

I have tested this item ✅ successfully on 9bbe5e6

This patch works as described.

After applying the patch a new option appears in the User Profile. When "Show Relative Dates in Lists = NO" is selected, the dates in the action logs appear as absolute dates. Otherwise they appear as relative dates.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33023.}

Co-authored-by: Quy <quy@fluxbb.org>

dgrammatiko · 2021-04-10T16:20:09Z

@Harmageddon sorry but this is not quite ok. Do NOT use HTMLHelper here, use JLayout, eg create a file in layouts/joomla/html/relative-date.php with content:

<?php
/**
 * @package     Joomla.Site
 * @subpackage  Layout
 *
 * @copyright   (C) 2016 Open Source Matters, Inc. <https://www.joomla.org>
 * @license     GNU General Public License version 2 or later; see LICENSE.txt
 */

use Joomla\CMS\HTML\HTMLHelper;
use Joomla\CMS\Language\Text;

defined('_JEXEC') or die;

extract($displayData);

/**
 * Layout variables
 * -----------------
 * @var   boolean  $forceRelative
 * @var   string  $format
 * @var   string  $date
 * @var   string  $showAbsoluteDate
 * @var   string  $unit
 * @var   string  $time
 */

$user = Factory::getApplication()->getIdentity();

if ($user === null)
{
	$useRelative = true;
}
else
{
	$useRelative = $forceRelative || $user->getParam('use_relative_dates', true);
}

// Format for the full / absolute date display.
$formatAbsolute = Text::_('DATE_FORMAT_LC6');

if (!$useRelative && $format === $formatAbsolute)
{
	return HTMLHelper::_('date', $date, $format);
}

$dateMain = $useRelative ? HTMLHelper::_('date.relative', $date, $unit, $time, $format) : HTMLHelper::_('date', $date, $format);
$dateAbsolute = HTMLHelper::_('date', $date, $formatAbsolute);

if ($showAbsoluteDate === 'tooltip')
{
	echo '<span>' . $dateMain . '</span><time role="tooltip">' . $dateAbsolute . '</time>';
}

if ($showAbsoluteDate === 'below')
{
	echo $dateMain . '<time class="small d-block">' . $dateAbsolute . '</time>';
}

echo $dateMain;

and then call it like:

<?php echo \Joomla\CMS\Layout\LayoutHelper::render(
	'joomla.html.relative-date',
	[
		'date'   => $item->log_date,
		'format' => Text::_('DATE_FORMAT_LC5'),
		$showAbsoluteDate = 'tooltip',
		$time = $now,
		$forceRelative = false,
		$unit = null
	]
); ?>

The reason is simple: using JLayout is easily overridable for front end devs. HTMLHelper need a plugin in order to be overriden

dgrammatiko · 2021-04-10T16:25:15Z

Actually, reading #33023 (comment) I think the idea of processing this data in the server is wrong, should be done on the client-side using Temporal: https://github.com/tc39/proposal-temporal and a simple web component.

Sorry, the JS is not temporal but https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/RelativeTimeFormat

Actually @PhilETaylor I think your PR that introduced the relative dates is wrong, we can do this using the browser native power... ( https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/RelativeTimeFormat
)

Quy · 2021-04-10T16:36:32Z

administrator/components/com_privacy/tmpl/consents/default.php

 								<?php echo $item->body; ?>
 							</td>
 							<td class="break-word">
-								<?php echo HTMLHelper::_('date.relative', new Date($item->created), null, $now); ?>


Please delete use Joomla\CMS\Date\Date;

michwebdev · 2021-04-23T19:04:49Z

I have tested this item ✅ successfully on 9ccc859

This worked. I had to learn about setting up a menu item for Privacy --> Create Request.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33023.}

obuisard · 2021-04-23T19:15:48Z

I have tested this item ✅ successfully on 9ccc859

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33023.}

richard67 · 2021-04-23T21:27:39Z

It's still draft and so tests don't count.

richard67 · 2021-04-23T21:28:54Z

@Harmageddon @dgrammatiko Is there anything left to be done or clarified regarding the above discussions? Shall I set the "Updates Requested" label?

dgrammatiko · 2021-04-24T11:21:36Z

Is there anything left to be done or clarified regarding the above discussions?

I'll stick to my opinion that this should NOT be an HTMLHelper thing but a JLayout, HTMLHelper should be used only for PHP functionality, if anything is echoed then that should be done directly with Layouts

Harmageddon · 2021-04-24T15:11:08Z

I see the point in making it a layout, but haven't found the time yet to convert and test it. I'm not sure about the web component proposal though, because apart from shifting the translation from our own translation files to the browser, I don't see great advantages, as we need to do the same calculations as far as I understand.

Thank you for testing this already, but as @richard67 said, this is still in draft state because of the discussions and open points listed above. As soon as it is ready for testing, I'm going to mark it as such.

Shall I set the "Updates Requested" label?

Does that make any difference while the PR is in draft state? If yes, feel free to do so.

richard67 · 2021-04-24T15:12:28Z

Does that make any difference while the PR is in draft state?

Right, draft state is sufficient.

richard67 · 2021-05-12T12:39:00Z

@Harmageddon @wilsonge @bembelimen Would it make sense to move this PR to 4.1?

Harmageddon · 2021-05-16T13:46:51Z

@Harmageddon @wilsonge @bembelimen Would it make sense to move this PR to 4.1?

As I sadly don't have the time right now to proceed with this, I assume it's not going to make it into 4.0. So yes, I'd re-target this for 4.1 and come up with a more consistent and overridable solution when I find the time.

HLeithner · 2022-06-27T13:08:30Z

This pull request has automatically rebased to 4.2-dev.

drmenzelit · 2022-10-21T11:44:10Z

The feature is interesting, but due to inactivity we are closing this draft. You can open it again, when you have the time to rework it.

brianteeman · 2022-10-21T12:03:25Z

@rdeutz

#PROD2020/024 - There shall be no indiscriminate closing of Issues or Pull Requests
Closing of Issues and Pull requests PR's shall NOT be done without underlying rationale and individual reasoning in the Issue or PR. Elapsing of a time period, inactivity of the submitter are examples that do NOT constitute valid reasons in and by themselves.

rdeutz · 2022-10-21T12:40:18Z

This motion is undicided and the maintainers have decided how we will do the Handling of abandoned PR’s.

The maintainers team will check older (older means, no relevant activity for a certain time frame, 18 months) Pull Requests and discuss if a Pull Request should be closed.

brianteeman · 2022-10-21T12:46:20Z

This motion is undicided and the maintainers have decided how we will do the Handling of abandoned PR’s.

My apologies - I missed thaat part

Harmageddon added 3 commits April 5, 2021 17:02

More flexible display of relative dates.

fba9702

Removed unnecessary parameter.

9b15d97

Added unit test for relativeFormatted method.

01e9992

joomla-cms-bot added Language Change This is for Translators PR-4.0-dev Unit/System Tests labels Apr 5, 2021

Harmageddon changed the title ~~More flexible display of relative dates.~~ [4.x] More flexible display of relative dates. Apr 5, 2021

Harmageddon marked this pull request as draft April 5, 2021 17:30

Worked on my machine (tm).

97e0442

brianteeman reviewed Apr 6, 2021

View reviewed changes

administrator/components/com_users/forms/user.xml Outdated Show resolved Hide resolved

This comment was marked as abuse.

Sign in to view

Harmageddon and others added 2 commits April 6, 2021 18:47

Apply suggestions from code review

243aadf

Co-authored-by: Brian Teeman <brian@teeman.net>

Don't use bootstrap tooltips.

952f320

Forgot about the unit test.

9bbe5e6

This comment was marked as abuse.

Sign in to view

Quy reviewed Apr 6, 2021

View reviewed changes

dgrammatiko reviewed Apr 7, 2021

View reviewed changes

Harmageddon and others added 3 commits April 10, 2021 14:36

Apply suggestion from code review

f06b4f7

Co-authored-by: Quy <quy@fluxbb.org>

Use <time> instead of <div>.

a21d63a

Changed order of parameters.

9ccc859

Quy reviewed Apr 10, 2021

View reviewed changes

This comment was marked as abuse.

Sign in to view

Harmageddon changed the base branch from 4.0-dev to 4.1-dev May 16, 2021 13:47

Quy added PR-4.1-dev and removed PR-4.0-dev labels Jun 13, 2021

HLeithner changed the base branch from 4.1-dev to 4.2-dev June 27, 2022 13:08

rdeutz added PR-4.2-dev and removed PR-4.1-dev labels Aug 1, 2022

drmenzelit closed this Oct 21, 2022

		$date, $unit = null, $time = null, $format = null, $forceRelative = false, $showAbsoluteDate = 'tooltip'
		)

Uh oh!

Conversation

Harmageddon commented Apr 5, 2021

Summary of Changes

Testing Instructions

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

With "Show Relative Dates in Lists" enabled

With "Show Relative Dates in Lists" disabled

Documentation Changes Required

Open Discussion Points

Uh oh!

Uh oh!

brianteeman commented Apr 6, 2021

Uh oh!

This comment was marked as abuse.

brianteeman commented Apr 6, 2021

Uh oh!

This comment was marked as abuse.

This comment was marked as abuse.

brianteeman commented Apr 6, 2021

Uh oh!

ChristineWk commented Apr 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as abuse.

Harmageddon commented Apr 6, 2021

Uh oh!

ChristineWk commented Apr 6, 2021

Uh oh!

This comment was marked as abuse.

ChristineWk commented Apr 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as abuse.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as abuse.

Uh oh!

Harmageddon Apr 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BertaOctech commented Apr 8, 2021

Uh oh!

dgrammatiko commented Apr 10, 2021

Uh oh!

dgrammatiko commented Apr 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michwebdev commented Apr 23, 2021

Uh oh!

obuisard commented Apr 23, 2021

Uh oh!

richard67 commented Apr 23, 2021

Uh oh!

richard67 commented Apr 23, 2021

ChristineWk commented Apr 6, 2021 •

edited

Loading

ChristineWk commented Apr 6, 2021 •

edited

Loading

Harmageddon Apr 10, 2021 •

edited

Loading

dgrammatiko commented Apr 10, 2021 •

edited

Loading