Decouple time widgets by Jan-Schuppik · Pull Request #350 · Icinga/ipl-web

Jan-Schuppik · 2026-01-08T11:46:30Z

This PR removes the direct dependency of the time widgets on Icinga Web 2 (\Icinga\Date\DateFormatter::formatDateTime()) and introduces a native relative-time formatting behavior in IPL Web.

Resolve:

Decouple TimeAgo, TimeUntil and TimeSince from Icinga Web #341

Depends on:

Provide Icinga to ipl/web behaviors icingaweb2#5238
(brings the ability to create js-behaviors outside of icingaweb2 )

Create a general Time Widget which is also the base for TimeAgo, TimeSince & TimeUntil

For this change to work the following PR is required: Icinga/icingaweb2#5238

src/Widget/Time.php

This is not the right place for timezone handling. It is also unnecessary for daylight saving time, because IANA time zones take care of the shift.

This reverts commit 7f1ab16.

Passing a timezone to `new DateTime()` when the input is a Unix timestamp prefixed with `@` is silently ignored by PHP — the timezone is always set to UTC in that case. Fix by calling `setTimezone()` explicitly after construction.

Instead of stripping the sign with Math.abs() before rendering and handling negation inconsistently across different calls, pass the raw signed value through and derive the display sign in render()

The until -> ago transition relies on a cached TimeAgo format. If none is cached yet, the transition silently fails. Fix by pre-assigning the ago label to TimeUntil widgets when they are animated (< 1h), ensuring the format is available when the transition occurs.

No code path in Time::diff() ever returns the DATETIME type, making the constant and its references in the subclass format maps dead code.

…tive sign

The ago-label is required on TimeAgo widgets with a sub-hour difference so users can toggle between relative and absolute time. The shared format string has been extracted into getRelativeTimeFormat() to avoid duplication with TimeUntil, and the JS-side deletion of data-ago-label has been removed so the label persists for subsequent toggles.

Replace deferred N_() string marking with direct t() calls to ensure strings are translated with the correct context. Also removes TimeAgo::getRelativeTimeFormat(), which made the translation context harder to trace for translators.

nilmerg

I stopped reading the tests once I saw more than one skip due to when the test is performed. Sorry, but this just non-sense. Tests must always succeed.

In addition, they make too heavy usage of contains and regex assertions. They test a HTML widget, so should assert the HTML structure: $obj actual, $html expected. Other tests exactly work like this and there is even a predefined assertion as part of ipl\Tests\Html\TestCase.

So please refactor your tests accordingly.

nilmerg · 2026-03-16T09:59:54Z

src/Widget/Time.php

+     *
+     * @throws \Exception
+     */
+    protected function castToDateTime(int|float|DateTime|null $value = null): DateTime


Please tag this as @internal so we have the option to remove it.

nilmerg · 2026-03-16T10:00:55Z

src/Widget/Time.php

+     */
+    protected function format(): string
+    {
+        return $this->timeString;


Please allow customizing this, by introducing a setter: setFormatter(IntlDateFormatter $formatter): static

nilmerg · 2026-03-16T15:12:47Z

src/Widget/Time.php

+     *
+     * @param DateTime $time
+     *
+     * @return array [string<formattedTime>, int<type>, DateInterval]


Suggested change

* @return array [string<formattedTime>, int<type>, DateInterval]

* @return array{0: string, 1: self::TIME|self::DATE|self::RELATIVE, 2: DateInterval}

https://phpstan.org/writing-php-code/phpdoc-types#array-shapes

nilmerg · 2026-03-16T15:28:57Z

src/Widget/TimeAgo.php

+        $onMessage = t('on %s', 'An event happened on the given date or date and time');
+        $map = [
+            self::RELATIVE => t('%s ago', 'An event that happened the given time interval ago'),
+            self::TIME     => t('at %s', 'An event happened at the given time'),
+            self::DATE     => null,
+        ];

-        $this->addAttributes([
-            'datetime' => $dateTime,
-            'title'    => $dateTime
-        ]);
+        [$time, $type] = $this->diff($this->dateTime);

-        $this->add(DateFormatter::timeAgo($this->ago));
+        return sprintf($map[$type] ?? $onMessage, $time);


I suppose this can be simplified using a match expression. In case diff() returns other types this needs to change anyway and a failing match expression makes it clear.

nilmerg · 2026-03-16T15:34:17Z

src/Widget/TimeAgo.php

+        [, , $interval] = $this->diff($this->dateTime);
+
+        $attributes = [
+            'datetime'           => $this->timeString,
+            'data-relative-time' => 'ago'
+        ];
+
+        if ($interval->days === 0 && $interval->h === 0) {
+            $attributes['data-ago-label'] = sprintf(
+                t('%s ago', 'An event that happened the given time interval ago'),
+                '0m 0s'
+            );
+        }
+
+        $this->addAttributes(Attributes::create($attributes));


Since the last step here doesn't differ from the base implementation but this needs to access the interval of the diff, did you consider performing all steps in format() and don't override assemble()? Right now it calls diff() twice after all.

nilmerg · 2026-03-16T15:35:42Z

src/Widget/Time.php

+
+    /**
+     * Get formatted time
+     *


I think this should mention that it is called only during assembly of the widget.

nilmerg · 2026-03-16T15:36:25Z

src/Widget/TimeSince.php

What I've commented for TimeAgo also applies here.

nilmerg · 2026-03-16T15:36:36Z

src/Widget/TimeUntil.php

What I've commented for TimeAgo also applies here.

nilmerg · 2026-03-16T15:38:26Z

tests/Widget/TimeAgoTest.php

+    if (! function_exists('ipl\Web\Widget\t')) {
+        function t(string $message, ?string $context = null): string
+        {
+            return $message;
+        }
+    }
+
+    if (! function_exists('ipl\Web\Widget\N_')) {
+        function N_(string $message): string
+        {
+            return $message;
+        }
+    }


The reason you need this, is that you use non-testable global functions in your implementation. Use the Translation trait instead and your test cases just need to setup an NoopTranslator just like other tests do it already.

nilmerg · 2026-03-16T15:41:13Z

tests/Widget/TimeTest.php

+        {
+            $now = new DateTime();
+            if ((int) $now->format('H') < 3) {
+                $this->markTestSkipped('Unreliable before 03:00 local time');


Tests relying on the current local time… Fake the time for …'s sake. 🙏

Allow the Time widget to accept a custom IntlDateFormatter instance, enabling locale-aware and customizable time formatting. Falls back to the default formatted datetime string when no formatter is set. Also declare the ext-intl PHP extension as an explicit dependency in composer.json.

PHPStan checks that the values in specified keys have the correct types. source: https://phpstan.org/writing-php-code/phpdoc-types#array-shapes

Remove the separate assemble() override in TimeAgo, TimeSince and TimeUntil. Attribute setup and text formatting are now handled entirely within format(), eliminating the duplicate diff() call and reducing the number of methods subclasses need to override.

@throws

…ocks Replace \Exception with the imported Exception class across Time and its subclasses, and add the missing Exception import and @throws annotation to Time::relative().

Previously, Time::diff() and Time::relative() always used new DateTime() as the reference point, making deterministic unit tests impossible. Introduce an optional $compareTime parameter throughout the widget family so callers can inject an explicit reference time: - Time::diff() accepts ?DateTime $compareTime instead of computing "now" internally; Time::relative() forwards it to TimeAgo/TimeUntil. - TimeAgo, TimeSince, and TimeUntil store the injected time in the new Time::$compareTime property. - Year/day comparisons using date('Y')/date('d') are replaced by the new isSameByFormat() helper, removing the last implicit "now" dependencies. - data-relative-time attributes are moved to $defaultAttributes, removing duplicate code from format() methods. - Fix inverted prefix logic in TimeUntil: only prepend '-' when invert is 0 and the interval is non-zero.

- Move all test classes out of namespace blocks into proper ipl\Tests\Web\Widget namespace - Extend TestCase instead of PHPUnit\Framework\TestCase directly - Initialize StaticTranslator with NoopTranslator in setUp() to avoid translation side effects - Consolidate redundant constructor tests (int/float/DateTime variants) into fewer, clearer cases - Rename test methods to focus on observable behavior rather than input type (e.g. testConstructorAcceptsIntTimestamp → testConstructorAcceptsTimestamp) - Add $compareTime parameter usage to pin time in tests for deterministic results

BastianLedererIcinga

I see my icingadb-web PR is still not entirely compatible with your changes, since I see no proper way to include a data-ago-label in Time widgets, which is required for the switch from absolute to relative. Let's discuss a solution and this time document the agreed approach in this PR.

BastianLedererIcinga · 2026-04-01T06:15:30Z

asset/js/behavior/RelativeTimeBehavior.js

@@ -0,0 +1,34 @@
+/* Icinga Web 2 | (c) 2025 Icinga GmbH | GPLv2+ */


This is no longer part of Icinga Web 2.

BastianLedererIcinga · 2026-04-01T09:45:29Z

asset/js/widget/RelativeTime.js

+            let match = TIME_REGEX_FULL.exec(content) || TIME_REGEX_MIN_ONLY.exec(content);
+
+            const template = {
+                prefix: (match?.[1]).replace(/-+$/g, '') ?? '',


If match is null replace() is called on undefined, so you need a second check if you want to be safe.

BastianLedererIcinga · 2026-04-01T10:44:19Z

asset/js/widget/RelativeTime.js

+            };
+
+            return fromTextContent(element, future);
+            // return fromDateTimeWithTimezone(element, future);


If there are no plans to use this please remove it and the dead code referenced by it.

BastianLedererIcinga · 2026-04-01T12:35:47Z

tests/Widget/TimeAgoTest.php

+        $this->assertSame($html, $rendered2);
+    }
+
+    public function testRenderHasAttributes(): void


Please refactor the tests as @nilmerg suggested. This entire test, testRenderHasAgoLabel and testRenderUsesTimeHtmlTag become superfluous once assertHtml is used.

Jan-Schuppik added 2 commits January 8, 2026 12:39

Introduce independ time-widgets

9da1580

Create a general Time Widget which is also the base for TimeAgo, TimeSince & TimeUntil

Add data-relative-time-attribute

5a2b6dd

cla-bot bot added the cla/signed label Jan 8, 2026

Jan-Schuppik added 2 commits January 8, 2026 12:51

Fix linting errors

97e7867

Add relative-time behavior (WIP)

ab8aa8e

BastianLedererIcinga mentioned this pull request Jan 19, 2026

Allow switching between relative and absolute timestamps Icinga/icingadb-web#1322

Open

Jan-Schuppik added 5 commits January 20, 2026 13:36

Fix missing timezone handling in relative-time behavior

c620465

cleanup time widgets

31cdf5e

Create a relative-time behavior as expected

29bdf2a

For this change to work the following PR is required: Icinga/icingaweb2#5238

Fix timezone handling - use available feature

4207ad3

Cleanup: relative-time-behavior

a1019c7

Jan-Schuppik mentioned this pull request Jan 21, 2026

Move relative time js behavior to ipl-web Icinga/icingaweb2#5461

Open

Jan-Schuppik added 3 commits January 21, 2026 20:09

Split relative-time in core and behavior

0072131

Remove notjQuery and simplify RelativeTime parameter

2f3d7b5

Add timezone to Time-widget

8ac3881

Jan-Schuppik requested a review from sukhwinder33445 January 22, 2026 14:34

Jan-Schuppik added 3 commits January 22, 2026 15:37

Add tryout: timezone refreshing mechanism

db2b8f5

Adjust: get timezone from icinga config in js

6d17781

Cleanup: code style & variable names

c8fd09a

sukhwinder33445 reviewed Jan 23, 2026

View reviewed changes

src/Widget/Time.php Outdated Show resolved Hide resolved

src/Widget/Time.php Outdated Show resolved Hide resolved

src/Widget/Time.php Outdated Show resolved Hide resolved

Jan-Schuppik added 10 commits January 27, 2026 13:54

Apply suggestions: adjust phpdocs of Time.php

9ef3790

Add: auto-refresh for timezones in js

b5cba9b

Adjust: rendering of the timestrings

216f9d2

Introduce: caching of timeformat

ff5030d

Adjust calculating the relative time

cd29ec9

Add caching of timezone offset

4ec6e58

Add cache for offset and element time differences

7d6db77

Remove element time difference cache (it slows down)

434f792

Fix: refresh cache on update & remove minus sign prefix

647c17e

Remove timezone handling

89c3d0e

This is not the right place for timezone handling. It is also unnecessary for daylight saving time, because IANA time zones take care of the shift.

Jan-Schuppik added 10 commits March 2, 2026 17:49

Revert "Refactor: remove t() and N_() namespace workaround from tests"

51b7a30

This reverts commit 7f1ab16.

Fix: linting error

9b93fd5

fix(RelativeTime): preserve sign through render pipeline

000ed5e

Instead of stripping the sign with Math.abs() before rendering and handling negation inconsistently across different calls, pass the raw signed value through and derive the display sign in render()

test: add tests to verify the ago label is set properly

639d72e

fix: remove unused DATETIME constant

2e23df8

No code path in Time::diff() ever returns the DATETIME type, making the constant and its references in the subclass format maps dead code.

fix: restrict TIME_REGEX_DIGITS to m/s units and capture leading nega…

8ff1022

…tive sign

fix and simplify time-format translation calls

3dea0ed

Replace deferred N_() string marking with direct t() calls to ensure strings are translated with the correct context. Also removes TimeAgo::getRelativeTimeFormat(), which made the translation context harder to trace for translators.

nilmerg requested changes Mar 16, 2026

View reviewed changes

Jan-Schuppik added 9 commits March 17, 2026 22:25

Mark helper method Time::castToDateTime() with @internal

15c594c

Use strong types in phpdoc for array shape checking

8336720

PHPStan checks that the values in specified keys have the correct types. source: https://phpstan.org/writing-php-code/phpdoc-types#array-shapes

Simplify Time::diff() by using match expression

c114506

Simplify time widgets formatting by using match expression

d3345f3

Use imported Exception instead of fully qualified \Exception in docbl…

2c2a8ec

…ocks Replace \Exception with the imported Exception class across Time and its subclasses, and add the missing Exception import and @throws annotation to Time::relative().

Set datetime attribute in base class instead of subclasses

cabdb2d

Jan-Schuppik force-pushed the decouple-time-widgets branch 2 times, most recently from 6d8dbd0 to c467617 Compare March 23, 2026 11:54

Jan-Schuppik added 2 commits March 23, 2026 13:12

Use Translation trait in time widgets for NoopTranslator testability

236d611

Time::relative() current time returns TimeAgo instead of TimeUntil

1c02bcc

Jan-Schuppik force-pushed the decouple-time-widgets branch from c467617 to 65cf226 Compare March 23, 2026 12:13

Jan-Schuppik force-pushed the decouple-time-widgets branch from 65cf226 to 7add9f3 Compare March 23, 2026 12:48

nilmerg requested a review from BastianLedererIcinga March 31, 2026 12:45

BastianLedererIcinga reviewed Apr 1, 2026

View reviewed changes

BastianLedererIcinga mentioned this pull request Apr 2, 2026

Fix incorrect start times in the timelines of fixed downtimes Icinga/icingadb-web#1355

Merged

	* @return array [string<formattedTime>, int<type>, DateInterval]
	* @return array{0: string, 1: self::TIME\|self::DATE\|self::RELATIVE, 2: DateInterval}

		@@ -0,0 +1,34 @@
		/* Icinga Web 2 \| (c) 2025 Icinga GmbH \| GPLv2+ */

Conversation

Jan-Schuppik commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nilmerg left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BastianLedererIcinga left a comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Jan-Schuppik commented Jan 8, 2026 •

edited

Loading