Review PR #1

MartinSchoeler · 2025-05-27T14:52:31Z

Let's use this PR to start the review process of what we currently have

…ion hook

MartinSchoeler · 2025-05-29T22:55:05Z

apps/meteor/client/startup/renderTimestamp.ts

+    'renderMessage',
+    (message: IMessageWithHTML): IMessageWithHTML => {
+        if (message.html) {
+            message.html = renderTimestamp(message.html);


@jhanyu2019 could you explain a bit the need for changing the message html using the callbacks?

Hi Martin, I use callbacks.add('renderMessage', ...) to register a high-priority callback that runs after the basic HTML of a message is generated. This callback inspects the message HTML and calls renderTimestamp to find and replace any <t:...> markup with a readable date/time string. This ensures timestamp markup is always rendered in a user-friendly format whenever messages are displayed.

Ah I see, as I've mentioned in another review, the parsing of the timestamps after the message was sent is already implemented, so I don't think we need to change anything related to message rendering for now

MartinSchoeler · 2025-06-01T19:16:02Z

packages/livechat/src/components/Messages/MessageText/markdown.ts

@@ -1,90 +1,138 @@
+/* eslint-disable import/no-unresolved */


I don't think these changes are need as we already have timestamp rendering for the messages once the timestamp feature preview has been enabled

Also, you can see that this component is in the livechat folder, livechat is a client used as an embedded chat popup. This client is not on the scope of the project and we can ignore these files for now

MartinSchoeler · 2025-06-01T19:20:44Z

apps/meteor/client/startup/renderTimestamp.ts

+    'renderMessage',
+    (message: IMessageWithHTML): IMessageWithHTML => {
+        if (message.html) {
+            message.html = renderTimestamp(message.html);


Ah I see, as I've mentioned in another review, the parsing of the timestamps after the message was sent is already implemented, so I don't think we need to change anything related to message rendering for now

MartinSchoeler · 2025-06-01T19:25:47Z

apps/meteor/client/components/message/toolbar/items/actions/Timestamp/TimestampButton.tsx

+			<MessageToolbarItem
+				id='timestamp'
+				icon='clock'
+				title={t('Add_Timestamp')}


I think this we could name it "Add Date and Time" or something similar, as we are not adding a timestamp anymore but an actual date and time that will be rendered to other users using the feature preview

MartinSchoeler · 2025-06-01T19:30:25Z

...eor/client/components/message/toolbar/items/actions/Timestamp/TimestampPicker/DatePicker.tsx

+			<Field>
+				<FieldLabel>{t('Date')}</FieldLabel>
+				<FieldRow>
+					<input


Here we can use the <InputBox> from fuselage

Here is the link to it's storybook https://rocketchat.github.io/fuselage/fuselage/main/?path=/docs/inputs-inputbox--docs

MartinSchoeler · 2025-06-01T19:35:35Z

apps/meteor/client/components/message/toolbar/items/actions/Timestamp/TimestampPicker/index.tsx

@@ -0,0 +1,65 @@
+import { Modal, Box } from '@rocket.chat/fuselage';


Let's create a storybook to this component, so later we can do some Jest Unit Snapshot tests. Snapshot tests are a good and "cheap" way to make sure the visual integrity of the component is properly covered by tests 😁

apps/meteor/client/components/message/toolbar/items/MobileItems.tsx

MartinSchoeler · 2025-06-01T20:07:05Z

apps/meteor/client/components/message/TimestampPicker.tsx

+    return `${year}-${month}-${day}`;
+};
+
+const TimestampPicker = ({ onClose, onInsert, initialDate = new Date() }: TimestampPickerProps) => {


What is the difference between this component and https://github.com/jhanyu2019/Rocket.Chat/pull/1/files#diff-0b68e36c275ada621778984cd276d827f1418ecd7e34a9bf9f477b8639126e63R16 ?

MartinSchoeler · 2025-06-01T20:10:19Z

apps/meteor/client/components/message/TimeInput.tsx

+    onChange: (value: string) => void;
+};
+
+const TimeInput = ({ value, onChange }: TimeInputProps) => {


What is the difference between this component and https://github.com/jhanyu2019/Rocket.Chat/pull/1/files#diff-0b68e36c275ada621778984cd276d827f1418ecd7e34a9bf9f477b8639126e63R16

packages/fuselage-ui-kit/tsconfig-esm.json

packages/ui-kit/tsconfig-esm.json

MartinSchoeler · 2025-06-22T17:41:16Z

...client/components/message/toolbar/items/actions/Timestamp/TimestampPicker/FormatSelector.tsx

+							value={selectedFormat} 
+							onChange={handleFormatChange}
+							options={formatOptions} 
+							style={{


Since the Select Component inherits from Box, you can just add the property width='full' to the component directly

MartinSchoeler · 2025-06-22T17:46:04Z

...client/components/message/toolbar/items/actions/Timestamp/TimestampPicker/FormatSelector.tsx

+			<Field>
+				<FieldLabel>{t('Format')}</FieldLabel>
+				<FieldRow>
+					<Box w='full'>


Thix box component is not needed here, just add the width='full' to the select component

MartinSchoeler · 2025-06-22T17:54:36Z

.../client/views/room/composer/messageBox/MessageBoxActionsToolbar/MessageBoxActionsToolbar.tsx

+    const createDiscussionAction = useCreateDiscussionAction(room); 
+    const shareLocationAction = useShareLocationAction(room, tmid); 
+	// Added for Timestamp feature
+    const timestampAction = useTimestampAction(chatContext.composer, !canSend || typing || isRecording);


The !canSend condition here stops the message action from adding timestamps if the message contains something, this check makes sense for actions like audioMessageAction since we cannot send text with the audios, but in the case of the timestamp, we should not check for these conditions

Suggested change

const timestampAction = useTimestampAction(chatContext.composer, !canSend || typing || isRecording);

const timestampAction = useTimestampAction(chatContext.composer);

MartinSchoeler · 2025-06-22T18:35:45Z

.../client/views/room/composer/messageBox/MessageBoxActionsToolbar/MessageBoxActionsToolbar.tsx

+        ...(!isHidden(hiddenActions, createDiscussionAction) && { createDiscussionAction }),
+        ...(!isHidden(hiddenActions, shareLocationAction) && { shareLocationAction }),
+        // Added for Timestamp feature
+        ...(!isHidden(hiddenActions, timestampAction) && { timestampAction}),


Hidden actions are set here https://github.com/jhanyu2019/Rocket.Chat/blob/feature/gsoc-timestamp-picker/apps/meteor/client/providers/LayoutProvider.tsx#L39 via browser message events. Ideally the timestamp picker action should be hidden when the feature preview is disabled.

MartinSchoeler · 2025-06-22T18:53:03Z

apps/meteor/client/lib/utils/timestamp/conversion.ts

+/**
+ * Formats a date as a relative time string
+ */
+const formatRelativeTime = (date: Date): string => {


This returns some weird values if the date is earlier than 1 year

The expected result for that date

I think we could experiment swapping the preview component for one of the Gazzodown components https://github.com/jhanyu2019/Rocket.Chat/blob/feature/gsoc-timestamp-picker/packages/gazzodown/src/elements/Timestamp/index.tsx#L39-L40

…t#36435) Co-authored-by: Guilherme Gazzo <[email protected]>

…tChat#36430)

This reverts commit ab9f0d0.

github-actions · 2025-07-17T13:50:07Z

PR Preview Action v1.6.2
🚀 View preview at https://jhanyu2019.github.io/Rocket.Chat/pr-preview/pr-1/
Built to branch `gh-pages` at 2025-08-03 05:25 UTC. Preview will be ready when the GitHub Pages deployment is complete.

Co-authored-by: Douglas Gubert <[email protected]>

…per-channel responses markdown (RocketChat#36386)

…36319) Co-authored-by: Tasso Evangelista <[email protected]>

…ketChat#36433) Co-authored-by: Diego Sampaio <[email protected]>

Co-authored-by: Douglas Gubert <[email protected]> Co-authored-by: Tasso Evangelista <[email protected]>

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

…ketChat#36465) Co-authored-by: Guilherme Gazzo <[email protected]> Co-authored-by: Diego Sampaio <[email protected]>

Co-authored-by: Diego Sampaio <[email protected]>

Co-authored-by: Guilherme Gazzo <[email protected]>

…at#36389)

Co-authored-by: Jéssica Souza <[email protected]> Co-authored-by: Matheus Cardoso <[email protected]> Co-authored-by: Guilherme Gazzo <[email protected]>

[no ci]

Release 7.9.0

[no ci]

Co-authored-by: Matheus Cardoso <[email protected]> Co-authored-by: Guilherme Gazzo <[email protected]>

…i18n support

…cketChat#36541)

…cketChat#35997)

…ketChat#36546) Co-authored-by: Douglas Fabris <[email protected]>

Co-authored-by: Tasso <[email protected]>

…en transfering (RocketChat#36577)

…#36564)

…cketChat#36574)

…time inputs

…estamp-picker

…napshots

Initial GSoC implementation: timestamp picker components and integrat…

258a799

…ion hook

MartinSchoeler commented May 29, 2025

View reviewed changes

MartinSchoeler commented Jun 1, 2025

View reviewed changes

Apply PR feedback: clean up duplicate code and apply additional fixes

2860374

MartinSchoeler commented Jun 22, 2025

View reviewed changes

jhanyu2019 and others added 7 commits July 1, 2025 01:13

Apply PR feedback: fix preview feature and address lint/stylelint issues

49ee488

sync tsconfig and ui-kit config files with upstream

ab9f0d0

chore: update PHP and SimpleSAMLphp versions in Dockerfile (RocketCha…

77ac153

…t#36435) Co-authored-by: Guilherme Gazzo <[email protected]>

fix: executeSaveUserProfile does not trigger username stream (Rocke…

4fda8af

…tChat#36430)

Revert "sync tsconfig and ui-kit config files with upstream"

08d3de2

This reverts commit ab9f0d0.

Merge branch 'develop' into feature/gsoc-timestamp-picker

b3e1b4a

fix: Fixes build & Page errors

2e3711d

MartinSchoeler and others added 17 commits July 17, 2025 14:39

feat: App Logs Export (RocketChat#36403)

6f20510

Co-authored-by: Douglas Gubert <[email protected]>

chore: remove hygen (RocketChat#36446)

302ef05

feat: add separateResponse param to incoming webhooks for optional …

9f3c11c

…per-channel responses markdown (RocketChat#36386)

regression: Remove pdf option in export messages as file (RocketChat#…

0d45149

…36319) Co-authored-by: Tasso Evangelista <[email protected]>

fix: do not resub router on message (RocketChat#36459)

6af403c

feat: use change streams to monitor instances (RocketChat#36451)

f40d7be

fix: await subscription updates in notifyUsersOnMessage function (Roc…

78ff5df

…ketChat#36433) Co-authored-by: Diego Sampaio <[email protected]>

feat: new App instances page & mixed app status (RocketChat#36359)

3f000d7

Co-authored-by: Douglas Gubert <[email protected]> Co-authored-by: Tasso Evangelista <[email protected]>

chore(deps): Upgrade date-fns (RocketChat#36415)

8078681

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

chore: Remove polyfills out of our support range (RocketChat#36428)

415d7c3

fix: await subscription updates in notifyUsersOnMessage function (Roc…

6a36b23

…ketChat#36465) Co-authored-by: Guilherme Gazzo <[email protected]> Co-authored-by: Diego Sampaio <[email protected]>

feat: use change streams to monitor instances (RocketChat#36462)

ced199f

Co-authored-by: Diego Sampaio <[email protected]>

fix: do not resub router on message (RocketChat#36461)

da6d6ae

Co-authored-by: Guilherme Gazzo <[email protected]>

regression: await user info retrieval in notifyOnUserChange (RocketCh…

0f552a5

…at#36389)

fix: redirect on required password change (RocketChat#36432)

796f238

Co-authored-by: Jéssica Souza <[email protected]> Co-authored-by: Matheus Cardoso <[email protected]> Co-authored-by: Guilherme Gazzo <[email protected]>

fix: resolve race condition affecting subscription stream updates

adb70bc

Release 7.8.1

0c317d9

[no ci]

ggazzo and others added 29 commits July 29, 2025 10:21

chore: bump version to 7.10.0-develop

8ad5fcd

chore: fix mocked license typo (RocketChat#36555)

74a162e

chore: correct version string formatting in rocketchat.info

470753a

test: new tests for isDepartmentCreationAvailable (RocketChat#36554)

8e00f78

feat: Add OpenAPI Support to chat.pinMessage API (RocketChat#36020)

c5f0be1

Release 7.9.0

2bccc5a

Release 7.9.0

Release 7.9.0

3bf0efe

[no ci]

fix: uikit-playground JSON editor not working (RocketChat#36563)

907d0d8

Merge remote-tracking branch 'origin/master' into develop

7d94976

feat: Add OpenAPI Support to oauth-apps.create API (RocketChat#36507)

f040b27

Co-authored-by: Matheus Cardoso <[email protected]> Co-authored-by: Guilherme Gazzo <[email protected]>

chore(deps): align typescript and jest versions (RocketChat#36561)

ce492b8

refactor: query ParentTeam data from reusable hooks (RocketChat#36562)

fc1270c

fix: update timezone logic, migrate useState to react-hook-form, add …

ea464f1

…i18n support

feat: adds new model function for aggregated agents by department (Ro…

84f7c43

…cketChat#36541)

chore: fix race condition on LocalBroker service's dependencies (Ro…

0776b21

…cketChat#35997)

i18n: Rocket.Chat language update from Lingohub 🤖 on 2025-07-28Z (Roc…

7a351d9

…ketChat#36546) Co-authored-by: Douglas Fabris <[email protected]>

chore: Users store (RocketChat#36289)

09b424b

Co-authored-by: Tasso <[email protected]>

fix: Remove room assignment before queueing the conversation again wh…

eed22e5

…en transfering (RocketChat#36577)

feat: Added annotation to WizardActions component (RocketChat#36575)

759b178

test: export messages with hide contextual bar preference (RocketChat…

0ca189f

…#36564)

refactor: enhance API error response validation and accessibility (Ro…

5f3ac3a

…cketChat#36574)

feat: App Logs Collapse All (RocketChat#36558)

8942187

fix: empty thread preview (RocketChat#36527)

e4a3b7c

fix: missing featured RoomToolBoxAction order (RocketChat#36591)

1a23b72

test: add TimestampPicker test and improve robustness for empty date/…

c4ed23d

…time inputs

Merge remote-tracking branch 'upstream/develop' into feature/gsoc-tim…

47128d2

…estamp-picker

chore: update snapshot

6175a0e

test: add accessibility tests and use jest fake timers to stabilize s…

2c0e383

…napshots

fix: update snapshot after merge conflict

025ee57

MartinSchoeler closed this Aug 7, 2025

		@@ -0,0 +1,65 @@
		import { Modal, Box } from '@rocket.chat/fuselage';

	const timestampAction = useTimestampAction(chatContext.composer, !canSend \|\| typing \|\| isRecording);
	const timestampAction = useTimestampAction(chatContext.composer);

Review PR #1

Review PR #1

Uh oh!

Conversation

MartinSchoeler commented May 27, 2025

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

github-actions bot commented Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Built to branch gh-pages at 2025-08-03 05:25 UTC. Preview will be ready when the GitHub Pages deployment is complete.

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

19 participants

github-actions bot commented Jul 17, 2025 •

edited

Loading

Built to branch `gh-pages` at 2025-08-03 05:25 UTC.
Preview will be ready when the GitHub Pages deployment is complete.