Scheduled post indicator #8522

harshilsharma63 · 2025-01-27T12:52:05Z

Summary

Added scheduled post indicator for mobile

Ticket Link

Fixes https://mattermost.atlassian.net/browse/MM-62284

Checklist

Added or updated unit tests (required for all new features)
Has UI changes
Includes text changes and localization file updates
Have tested against the 5 core themes to ensure consistency between them.
Have run E2E tests by adding label E2E iOS tests for PR.

Device Information

This PR was tested on: iOS emulator

Screenshots

Release Note

None

…ost_indicator

harshilsharma63 · 2025-01-28T11:06:42Z

app/components/scheduled_post_indicator/scheduled_post_indicator.tsx

+    const theme = useTheme();
+    const styles = getStyleSheet(theme);
+
+    const [scheduledPostCount] = React.useState(125);


This 125 is hardcoded just for demo. It will later be replaced with actual count.

In this PR, or in a future PR?

In the next week or two. It will be in there before we merge feature branch into main.

Please put a TODO comment that this value needs to be replaced. Just so that it won't get forgotten.

I'll work on integrating this today in a separate PR.

scheduledPostCount is now. props, so this is removed now.

rahimrahman

I've "accidentally" reviewed this yesterday but realized it was a draft.

rahimrahman · 2025-01-27T18:38:25Z

ios/Gekidou/Package.resolved

@@ -1,13 +1,94 @@
 {
  "object": {
    "pins": [
+      {


Please revert changes to this file.

The changes in this file is still showing up. Can you make sure it's fully reverted?

rahimrahman · 2025-01-27T18:39:04Z

assets/base/i18n/en.json

+  "scheduled_post.channel_indicator.link_to_scheduled_posts.text": "See all.",
+  "scheduled_post.channel_indicator.multiple": "{count} scheduled messages in channel.",
+  "scheduled_post.channel_indicator.single": "Message scheduled for {dateTime}.",


Do we need the .? I don't see any other messages having dots.

There are a lot of messages ending in a full stop, such as settings_display.crt.desc, server.websocket.unreachable etc. If we need to display a full stop in UI, it has to go in here.

rahimrahman · 2025-01-27T18:39:26Z

app/screens/channel/channel.tsx

@@ -149,6 +150,8 @@ const Channel = ({
                            nativeID={channelId}
                        />
                    </View>
+                    {/*scheduled post indicator goes here*/}


Probably don't need the comments anymore.

rahimrahman · 2025-01-27T18:43:12Z

app/components/scheduled_post_indicator/scheduled_post_indicator.tsx

+    const theme = useTheme();
+    const styles = getStyleSheet(theme);
+
+    const [scheduledPostCount] = React.useState(125);


Completely arbitrary. This will be picked from local database once integrated with it.

rahimrahman · 2025-01-27T18:43:29Z

app/components/scheduled_post_indicator/scheduled_post_indicator.tsx

+            <FormattedTime
+                timezone={getUserTimezone(currentUser)}
+                isMilitaryTime={isMilitaryTime}
+                value={1768902936000}


why this value?

Completely arbitrary. This will be picked from local database once integrated with it.

Same, add a TODO as instruction to replace with proper value. It will stand up hopefully in the main PR.

rahimrahman · 2025-01-27T18:44:19Z

app/components/scheduled_post_indicator/scheduled_post_indicator.tsx

@@ -0,0 +1,107 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.


Don't forget unit test for this component.

After feature complete. Please be rest assured we will add the tests.

…ost_indicator

…reference

…onfigs

…t file

…Everything

…scenarios

rahimrahman · 2025-01-29T06:29:26Z

app/components/scheduled_post_indicator/scheduled_post_indicator.tsx

+            <FormattedTime
+                timezone={getUserTimezone(currentUser)}
+                isMilitaryTime={isMilitaryTime}
+                value={1768902936000}


Same, add a TODO as instruction to replace with proper value. It will stand up hopefully in the main PR.

rahimrahman · 2025-01-31T17:23:08Z

app/screens/channel/__snapshots__/channel.test.tsx.snap

@@ -0,0 +1,359 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP


How can there be a snapshot file when the channel.test.tsx doesn't have toMatchSnapshot()?

And thank you for not doing toMatchSnapshot().

I must have added it and removed it but didn't remove the snapshot file.

rahimrahman · 2025-01-31T17:25:32Z

app/components/scheduled_post_indicator/scheduled_post_indicator.test.tsx

+        expect(getByText(/See all./)).toBeTruthy();
+    });
+
+    it('should render multiple scheduled posts indicator for thread', async () => {


duplicate with above?

Nope, above one is for channels and this one is for threads. The text changes based on isThread prop, as being tested here.

rahimrahman · 2025-01-31T17:26:28Z

app/components/scheduled_post_indicator/scheduled_post_indicator.test.tsx

+        );
+
+        const timeElement = await screen.findByTestId('scheduled_post_indicator_single_time');
+        expect(timeElement).toBeTruthy();


Is it really a military time? Just knowing the testId is there enough to say that it is military time? Why?

Added check for it now.

rahimrahman · 2025-01-31T17:27:04Z

app/components/scheduled_post_indicator/scheduled_post_indicator.test.tsx

+        );
+
+        const timeElement = await screen.findByTestId('scheduled_post_indicator_single_time');
+        expect(timeElement).toBeTruthy();


Same as above. Just knowing the timeElement exists doesn't really tell me that it's regular 12-hour vs military time. Is there another way to prove this?

Added check for it now.

rahimrahman · 2025-01-31T17:27:25Z

app/components/scheduled_post_indicator/scheduled_post_indicator.test.tsx

+        );
+
+        const timeElement = await screen.findByTestId('scheduled_post_indicator_single_time');
+        expect(timeElement).toBeTruthy();


Same, but how would I know the user is missing?

Its just that it doesn't break when user is missing, no special behavior occurs on user missing that can be tested. Its a "just for safety" test, not useful.

rahimrahman · 2025-01-31T17:28:38Z

app/screens/channel/channel.tsx

@@ -149,6 +150,8 @@ const Channel = ({
                            nativeID={channelId}
                        />
                    </View>
+                    {/*This count is hardcoded but will be removed during integration work*/}


Can you add a TODO: on it?

I can, but the ESlint doesn't like TODOs in code, so I have to add {/* eslint-disable-next-line no-warning-comments */} to ignore that warning, so its not worth it. Its not like this will be missed, its a feature branch work. All PRs will be incomplete y themselves and we'll end up adding dozens of TODOs everywhere. Not worth it.

i've added the TODO and the corresponding eslint fix anyways. LMK if you want me to continue adding those.

rahimrahman · 2025-01-31T17:30:04Z

app/screens/channel/channel.test.tsx

+    it('does not render posts while switching channels', async () => {
+        const mockedChannelSwitch = jest.mocked(useChannelSwitch);
+        mockedChannelSwitch.mockReturnValue(true);
+
+        renderWithEverything(
+            <Channel {...getBaseProps()}/>,
+            {database},
+        );
+
+        expect(screen.queryByTestId('channel.post_draft')).toBeNull();


Duplicated above again?

rahimrahman · 2025-01-31T17:31:35Z

app/screens/channel/channel.test.tsx

+        jest.clearAllMocks();
+    });
+
+    it('does not render posts while switching teams', async () => {


Does not render post or does not render scheduled post or draft?

rahimrahman · 2025-01-31T17:34:07Z

app/screens/channel/channel.test.tsx

@@ -0,0 +1,104 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.


This test file feels a little off. The two tests are duplicated?

And there are no story to tell here.

If a test is saying that there's no post when switching teams, can you show there are post when not switching team?

I guess there needs to be some sort of a base. Like what do I expect the channel file to display when it has basic props.

Then go from there...

I'm deleting this test for now, its a very large existing component and my changes in it are minor. Adding tests for the whole channel component is beyond the scope of this PR and feature.

rahimrahman · 2025-01-31T17:35:25Z

app/components/post_list/scroll_to_end_view.tsx

@@ -97,7 +97,7 @@ const ScrollToEndView = ({
        () => ({
            transform: [
                {
-                    translateY: withTiming(showScrollToEndBtn ? -80 - keyboardOverlap - bottomAdjustment : 0, {duration: 300}),
+                    translateY: withTiming(showScrollToEndBtn ? -100 - keyboardOverlap - bottomAdjustment : 0, {duration: 300}),


Change delta of "-20" has not much of a significant, but it would be nice to know why it was made. Diff in screenshot would help, but I'm happy with quick explanation.

This is how it looks with -80

This is how it looks with -100

adjustment of 20 is all we need. Maybe a few pixels here or there more, but thats to be done later during a UX review.

rahimrahman · 2025-01-31T17:37:20Z

app/components/scheduled_post_indicator/scheduled_post_indicator.tsx

+
+    let scheduledPostText: React.ReactNode;
+
+    if (scheduledPostCount <= 0) {


Qurious: Can scheduledPostCount ever be less than 0?

I accidentally set a negative value once, so I handled it. However, since it doesn’t make sense according to the specification, I replaced it with an “===“ case.

rahimrahman · 2025-01-31T17:38:11Z

app/components/scheduled_post_indicator/scheduled_post_indicator.tsx

+
+    if (scheduledPostCount <= 0) {
+        return null;
+    } else if (scheduledPostCount === 1) {


What happens if it's more than 1? You will not show dateTime at all?

Why not?

Correct, because we cannot show date and time of all scheduled posts in a small indicator in channel. See the attached screenshot in PR description for example.

…ost_indicator

larkox · 2025-02-04T08:49:31Z

@abhijit-singh There was a discussion not long ago about the amount of banners taking real state at the top of the screen, and this PR just made me realize we also have a lot of stuff happening at the bottom.

In particular... we can have someone typing in the channel, while we have scheduled messages, while we are writing a priority message, with several lines of text and a couple of attachments. Not sure from the top of my head if we have any other element living in that space.

Taking that into consideration, and also the existing banners on the top, are we ok with continuing with this approach for scheduled messages in the channel?

larkox · 2025-02-04T08:50:37Z

@harshilsharma63 Have you tested how well / bad this interact with the "User X is typing on the channel" message?

larkox

LGTM, just a couple questions and comments, but nothing really blocking.

larkox · 2025-02-04T08:52:30Z

app/components/post_list/scroll_to_end_view.tsx

The changes here seem to affect no matter if the scheduled post message is visible or not. Is that intended? Is there any visual change due to this?

It looks right both with and without the scheduled post indicator with the same change.

Here is a video of with and without scheduled post indicator. Maybe I'll need to add some extra spacing conditionally, but will figure it out later.

Screen.Recording.2025-02-04.at.4.42.06.PM.mov

Screen.Recording.2025-02-04.at.4.40.44.PM.mov

larkox · 2025-02-04T08:57:27Z

app/components/scheduled_post_indicator/scheduled_post_indicator.tsx

+                    <Text style={styles.link}>
+                        <FormattedMessage
+                            id='scheduled_post.channel_indicator.link_to_scheduled_posts.text'
+                            defaultMessage='See all.'
+                        />
+                    </Text>


What is the plan here? For this to be a touchable, or for the whole banner to be a touchable?

We are not yet implementing this part because we don't have the screen, right? Or am I missing something?

User will be able to click here to go the screen, which doesn't exist yet. hence, only text for now.

larkox · 2025-02-04T09:03:31Z

app/components/scheduled_post_indicator/scheduled_post_indicator.test.tsx

+        const timeElement = await findByTestId('scheduled_post_indicator_single_time');
+        expect(timeElement).toBeVisible();
+        expect(getByText(/Message scheduled for/)).toBeVisible();
+    });


There are a couple of prop that can be undefined that you are not testing: scheduledPostCount and isThread. isThread may not be needed, since undefined is falsy, and you are setting the default value for scheduledPostCount to 0, but I guess it wouldn't hurt to add test to make sure those defaults never change.

scheduledPostCount will default to 0 once I integrate it with local database and will never be undefined.

harshilsharma63 · 2025-02-04T11:07:25Z

LGTM, just a couple questions and comments, but nothing really blocking.

Great catch! I've filed https://mattermost.atlassian.net/browse/MM-62871 for addressing it.

* WIP * Separate component for core and custom options * WIP * WIP * WIP * unified options into single component * Scheduled post menu ready * Displayed date time picker by default * WIP * Lint fix * i18n fix * removed unused screen * API call sent successfully * Added user time format preference * review fixex and improvements * Handled loading sttae * Handled loading sttae * Handled mentions and files and priority * Cleanup * lint fix * i18n fix * WIP * Displayed error snackbar * Fixed time used for debugging * Lint fix * review fixex and improvements * Renamed a file * Adding tests * Adding tests * lint fix * cleanup * Added draft_input tests * test: Add comprehensive tests for SendAction component * Added send_action tests * lint fix * test: Add comprehensive test coverage for useHandleSendMessage hook * test: Add test for scheduled post creation in send message hook * fix: Mock createScheduledPost function in test to resolve matcher error * WIP tests * test: Add tests for SnackBar component * Added snack bar tests * Cleanup * test: Add empty test file for scheduled post picker * test: Add tests for ScheduledPostOptions component * test: Improve ScheduledPostOptions test coverage and error handling * test: Update ScheduledPostOptions test suite with new database and timezone mocking * test: Fix timeout issues in ScheduledPostOptions test suite * Added scheuidled post picker tests * test: Add empty test file for scheduled post footer * test: Add comprehensive tests for ScheduledPostFooter component * feat: Add animatedFooterPosition prop to ScheduledPostFooter test * test: Add tablet rendering test for ScheduledPostFooter * test: Update ScheduledPostFooter test mocks and remove unused imports * test: Add mocks for BottomSheet context in scheduled post footer tests * Added scheudled post footer test * test: Add comprehensive tests for handle_send_message hook * Enhanced handle send message tests * cleanup * Added missing snapshots * test: Add error logging and force logout validation for scheduled post tests * Updated test * used toBeVisible instead of toBeTruthy * test: Update test assertions to use toHaveBeenCalledWith * test: Add assertions to prevent sendMessage when send button is disabled * test: Replace snapshot tests with explicit UI element checks * test improvements * Deleted unused snapshots * Updated snaopshot * feat: Add close button ID for scheduled post options bottom sheet * test: Add mock scheduling flow to scheduled post options test * Fixed incorrect test * scheudled post test enhancement * minimised a test * Scheduled post indicator (#8522) * WIP * WIP * DFisplayed everything correctly * Lint fix * handled thread and padding * removed a comment * Fixed i18n * test: Add empty test file for scheduled post indicator component * test: Add tests for ScheduledPostIndicator component * test: Rewrite ScheduledPostIndicator tests with improved coverage * test: Update scheduled post indicator test setup with TestHelper * fix: Replace non-existent addPreferencesToServer with TestHelper.setPreference * refactor: Update ScheduledPostIndicator tests to use operator.handleConfigs * refactor: Use renderWithEverything and consistently pass database prop * Added tests for scheduled_post_indicator * test: Add channel screen test file * test: Add comprehensive tests for Channel component * refactor: Remove unused import and clean up whitespace in channel test file * refactor: Update Channel component tests with database and renderWithEverything * refactor: Update Channel test cases to remove async/await and add type * test: Wrap channel screen render in act() to handle async updates * fix: Resolve unmounted renderer error in Channel snapshot test * test: Remove unnecessary async act in channel test snapshot * fix: Resolve unmounted renderer issue in channel screen test * Added chanels test * test: Add empty test file for thread screen * test: Add tests for Thread component with rendering and call feature scenarios * cleanup * Mock up0date * temperorily removed a flaky test * review fixes * Fixed a test * Fixed a test * Fixed a test

harshilsharma63 added 4 commits January 27, 2025 10:16

WIP

23adb2c

WIP

e2661a4

DFisplayed everything correctly

62738c2

Lint fix

e8eafa9

mm-cloud-bot added the release-note-none label Jan 27, 2025

harshilsharma63 added 2 commits January 28, 2025 14:32

Merge branch 'create_scheduled_post_api_integration' into scheduled_p…

09c5cc2

…ost_indicator

handled thread and padding

a74d1c3

harshilsharma63 marked this pull request as ready for review January 28, 2025 11:05

harshilsharma63 requested review from rahimrahman and Rajat-Dabade January 28, 2025 11:06

harshilsharma63 commented Jan 28, 2025

View reviewed changes

removed a comment

821aa3f

rahimrahman reviewed Jan 28, 2025

View reviewed changes

harshilsharma63 added 2 commits January 29, 2025 11:47

Merge branch 'create_scheduled_post_api_integration' into scheduled_p…

9acdb90

…ost_indicator

Fixed i18n

c470968

harshilsharma63 requested a review from rahimrahman January 29, 2025 07:14

harshilsharma63 added 14 commits January 31, 2025 13:44

Merge branch 'create_scheduled_post_api_integration' into scheduled_p…

de651db

…ost_indicator

test: Add empty test file for scheduled post indicator component

f093a60

test: Add tests for ScheduledPostIndicator component

d3ef484

test: Rewrite ScheduledPostIndicator tests with improved coverage

7c3d9f5

test: Update scheduled post indicator test setup with TestHelper

0d319b3

fix: Replace non-existent addPreferencesToServer with TestHelper.setP…

079382b

…reference

refactor: Update ScheduledPostIndicator tests to use operator.handleC…

152bcf5

…onfigs

refactor: Use renderWithEverything and consistently pass database prop

94c6300

Added tests for scheduled_post_indicator

b061ccd

test: Add channel screen test file

25a19c7

test: Add comprehensive tests for Channel component

c0b0692

refactor: Remove unused import and clean up whitespace in channel tes…

c7e9132

…t file

refactor: Update Channel component tests with database and renderWith…

36f4861

…Everything

refactor: Update Channel test cases to remove async/await and add type

2c8179c

harshilsharma63 added 10 commits January 31, 2025 14:49

test: Wrap channel screen render in act() to handle async updates

7ffd151

fix: Resolve unmounted renderer error in Channel snapshot test

7ad4983

test: Remove unnecessary async act in channel test snapshot

90fc974

fix: Resolve unmounted renderer issue in channel screen test

9f892ca

Added chanels test

248c2fd

test: Add empty test file for thread screen

8937f57

test: Add tests for Thread component with rendering and call feature …

a01b17e

…scenarios

cleanup

d24c828

Mock up0date

44ac162

temperorily removed a flaky test

ba9053d

rahimrahman reviewed Jan 31, 2025

View reviewed changes

harshilsharma63 added 6 commits February 3, 2025 12:24

Merge branch 'create_scheduled_post_api_integration' into scheduled_p…

4aae8d5

…ost_indicator

review fixes

dc2e865

Merge branch 'create_scheduled_post_api_integration' into scheduled_p…

32777f2

…ost_indicator

Fixed a test

1015461

Fixed a test

78d5f45

Fixed a test

f49ab74

harshilsharma63 requested review from rahimrahman and larkox and removed request for Rajat-Dabade February 3, 2025 10:18

rahimrahman approved these changes Feb 4, 2025

View reviewed changes

larkox approved these changes Feb 4, 2025

View reviewed changes

harshilsharma63 merged commit cb2e36e into create_scheduled_post_api_integration Feb 4, 2025
6 checks passed

harshilsharma63 deleted the scheduled_post_indicator branch February 4, 2025 11:18

		@@ -0,0 +1,107 @@
		// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.

		@@ -0,0 +1,104 @@
		// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.


		let scheduledPostText: React.ReactNode;

		if (scheduledPostCount <= 0) {

Scheduled post indicator #8522

Scheduled post indicator #8522

Conversation

harshilsharma63 commented Jan 27, 2025

Summary

Ticket Link

Checklist

Device Information

Screenshots

Release Note

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harshilsharma63 Jan 29, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rahimrahman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rahimrahman Feb 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harshilsharma63 Feb 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harshilsharma63 Feb 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

larkox commented Feb 4, 2025

larkox commented Feb 4, 2025

larkox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harshilsharma63 commented Feb 4, 2025

harshilsharma63 Jan 29, 2025 •

edited

Loading

rahimrahman Feb 3, 2025 •

edited

Loading

harshilsharma63 Feb 3, 2025 •

edited

Loading

harshilsharma63 Feb 3, 2025 •

edited

Loading