Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scheduled post indicator #8522

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
23adb2c
WIP
harshilsharma63 Jan 20, 2025
e2661a4
WIP
harshilsharma63 Jan 21, 2025
62738c2
DFisplayed everything correctly
harshilsharma63 Jan 27, 2025
e8eafa9
Lint fix
harshilsharma63 Jan 27, 2025
09c5cc2
Merge branch 'create_scheduled_post_api_integration' into scheduled_p…
harshilsharma63 Jan 28, 2025
a74d1c3
handled thread and padding
harshilsharma63 Jan 28, 2025
821aa3f
removed a comment
harshilsharma63 Jan 28, 2025
9acdb90
Merge branch 'create_scheduled_post_api_integration' into scheduled_p…
harshilsharma63 Jan 29, 2025
c470968
Fixed i18n
harshilsharma63 Jan 29, 2025
de651db
Merge branch 'create_scheduled_post_api_integration' into scheduled_p…
harshilsharma63 Jan 31, 2025
f093a60
test: Add empty test file for scheduled post indicator component
harshilsharma63 Jan 31, 2025
d3ef484
test: Add tests for ScheduledPostIndicator component
harshilsharma63 Jan 31, 2025
7c3d9f5
test: Rewrite ScheduledPostIndicator tests with improved coverage
harshilsharma63 Jan 31, 2025
0d319b3
test: Update scheduled post indicator test setup with TestHelper
harshilsharma63 Jan 31, 2025
079382b
fix: Replace non-existent addPreferencesToServer with TestHelper.setP…
harshilsharma63 Jan 31, 2025
152bcf5
refactor: Update ScheduledPostIndicator tests to use operator.handleC…
harshilsharma63 Jan 31, 2025
94c6300
refactor: Use renderWithEverything and consistently pass database prop
harshilsharma63 Jan 31, 2025
b061ccd
Added tests for scheduled_post_indicator
harshilsharma63 Jan 31, 2025
25a19c7
test: Add channel screen test file
harshilsharma63 Jan 31, 2025
c0b0692
test: Add comprehensive tests for Channel component
harshilsharma63 Jan 31, 2025
c7e9132
refactor: Remove unused import and clean up whitespace in channel tes…
harshilsharma63 Jan 31, 2025
36f4861
refactor: Update Channel component tests with database and renderWith…
harshilsharma63 Jan 31, 2025
2c8179c
refactor: Update Channel test cases to remove async/await and add type
harshilsharma63 Jan 31, 2025
7ffd151
test: Wrap channel screen render in act() to handle async updates
harshilsharma63 Jan 31, 2025
7ad4983
fix: Resolve unmounted renderer error in Channel snapshot test
harshilsharma63 Jan 31, 2025
90fc974
test: Remove unnecessary async act in channel test snapshot
harshilsharma63 Jan 31, 2025
9f892ca
fix: Resolve unmounted renderer issue in channel screen test
harshilsharma63 Jan 31, 2025
248c2fd
Added chanels test
harshilsharma63 Jan 31, 2025
8937f57
test: Add empty test file for thread screen
harshilsharma63 Jan 31, 2025
a01b17e
test: Add tests for Thread component with rendering and call feature …
harshilsharma63 Jan 31, 2025
d24c828
cleanup
harshilsharma63 Jan 31, 2025
44ac162
Mock up0date
harshilsharma63 Jan 31, 2025
ba9053d
temperorily removed a flaky test
harshilsharma63 Jan 31, 2025
4aae8d5
Merge branch 'create_scheduled_post_api_integration' into scheduled_p…
harshilsharma63 Feb 3, 2025
dc2e865
review fixes
harshilsharma63 Feb 3, 2025
32777f2
Merge branch 'create_scheduled_post_api_integration' into scheduled_p…
harshilsharma63 Feb 3, 2025
1015461
Fixed a test
harshilsharma63 Feb 3, 2025
78d5f45
Fixed a test
harshilsharma63 Feb 3, 2025
f49ab74
Fixed a test
harshilsharma63 Feb 3, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/components/post_list/scroll_to_end_view.tsx
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const getStyleFromTheme = makeStyleSheetFromTheme((theme) => {
buttonStyle: {
position: 'absolute',
alignSelf: 'center',
bottom: -70,
bottom: -100,
flexDirection: 'row',
},
shadow: {
Expand Down Expand Up @@ -97,7 +97,7 @@ const ScrollToEndView = ({
() => ({
transform: [
{
translateY: withTiming(showScrollToEndBtn ? -80 - keyboardOverlap - bottomAdjustment : 0, {duration: 300}),
translateY: withTiming(showScrollToEndBtn ? -100 - keyboardOverlap - bottomAdjustment : -15, {duration: 300}),
},
],
maxWidth: withTiming(isNewMessage ? 169 : 40, {duration: 300}),
Expand Down
25 changes: 25 additions & 0 deletions app/components/scheduled_post_indicator/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

import {withDatabase, withObservables} from '@nozbe/watermelondb/react';
import {map} from 'rxjs/operators';

import {getDisplayNamePreferenceAsBool} from '@helpers/api/preference';
import {queryDisplayNamePreferences} from '@queries/servers/preference';
import {observeCurrentUser} from '@queries/servers/user';

import {ScheduledPostIndicator} from './scheduled_post_indicator';

const enhance = withObservables([], ({database}) => {
const currentUser = observeCurrentUser(database);
const preferences = queryDisplayNamePreferences(database).
observeWithColumns(['value']);
const isMilitaryTime = preferences.pipe(map((prefs) => getDisplayNamePreferenceAsBool(prefs, 'use_military_time')));

return {
currentUser,
isMilitaryTime,
};
});

export default withDatabase(enhance(ScheduledPostIndicator));
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

import {Database} from '@nozbe/watermelondb';
import {screen} from '@testing-library/react-native';
import React from 'react';

import NetworkManager from '@managers/network_manager';
import {renderWithEverything} from '@test/intl-test-helper';
import TestHelper from '@test/test_helper';

import ScheduledPostIndicator from './';

import type ServerDataOperator from '@database/operator/server_data_operator';

const SERVER_URL = 'https://appv1.mattermost.com';

// this is needed when using the useServerUrl hook
jest.mock('@context/server', () => ({
useServerUrl: jest.fn(() => SERVER_URL),
}));

describe('components/scheduled_post_indicator', () => {
let database: Database;
let operator: ServerDataOperator;

beforeEach(async () => {
const server = await TestHelper.setupServerDatabase(SERVER_URL);
database = server.database;
operator = server.operator;
});

afterEach(async () => {
await TestHelper.tearDown();
NetworkManager.invalidateClient(SERVER_URL);
});

it('should render single scheduled post indicator correctly', async () => {
const {getByText} = renderWithEverything(
<ScheduledPostIndicator
isThread={false}
scheduledPostCount={1}
/>,
{database},
);

await screen.findByTestId('scheduled_post_indicator_single_time');
expect(getByText(/Message scheduled for/)).toBeVisible();
expect(getByText(/See all./)).toBeVisible();
});

it('should render multiple scheduled posts indicator for channel', async () => {
const {getByText} = renderWithEverything(
<ScheduledPostIndicator
isThread={false}
scheduledPostCount={10}
/>,
{database},
);

expect(getByText(/10 scheduled messages in channel./)).toBeVisible();
expect(getByText(/See all./)).toBeVisible();
});

it('should render multiple scheduled posts indicator for thread', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicate with above?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

const {getByText} = renderWithEverything(
<ScheduledPostIndicator
isThread={true}
scheduledPostCount={10}
/>,
{database},
);

expect(getByText(/10 scheduled messages in thread./)).toBeVisible();
expect(getByText(/See all./)).toBeVisible();
});

it('renders with military time when preference is set', async () => {
await operator.handlePreferences({
preferences: [
{
user_id: 'user_1',
category: 'display_settings',
name: 'use_military_time',
value: 'true',
},
],
prepareRecordsOnly: false,
});

const {getByText, findByTestId} = renderWithEverything(
<ScheduledPostIndicator
scheduledPostCount={1}
/>,
{database},
);

const timeElement = await findByTestId('scheduled_post_indicator_single_time');
expect(timeElement).toBeVisible();

expect(getByText(/19:41/)).toBeVisible();
});

it('renders with 12-hour time when preference is set to 12 hours', async () => {
await operator.handlePreferences({
preferences: [
{
user_id: 'user_1',
category: 'display_settings',
name: 'use_military_time',
value: 'false',
},
],
prepareRecordsOnly: false,
});

const {getByText, findByTestId} = renderWithEverything(
<ScheduledPostIndicator
scheduledPostCount={1}
/>,
{database},
);

const timeElement = await findByTestId('scheduled_post_indicator_single_time');
expect(timeElement).toBeVisible();

expect(getByText(/7:41 PM/)).toBeVisible();
});

it('renders with 12-hour time when preference is not set', async () => {
const {getByText, findByTestId} = renderWithEverything(
<ScheduledPostIndicator
scheduledPostCount={1}
/>,
{database},
);

const timeElement = await findByTestId('scheduled_post_indicator_single_time');
expect(timeElement).toBeVisible();

expect(getByText(/7:41 PM/)).toBeVisible();
});

it('handles missing current user', async () => {
const {getByText, findByTestId} = renderWithEverything(
<ScheduledPostIndicator
scheduledPostCount={1}
/>,
{database},
);

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

});
130 changes: 130 additions & 0 deletions app/components/scheduled_post_indicator/scheduled_post_indicator.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget unit test for this component.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

// See LICENSE.txt for license information.

import React from 'react';
import {FormattedMessage} from 'react-intl';
import {Text, View} from 'react-native';

import CompassIcon from '@components/compass_icon';
import FormattedTime from '@components/formatted_time';
import {useTheme} from '@context/theme';
import {changeOpacity, makeStyleSheetFromTheme} from '@utils/theme';
import {getUserTimezone} from '@utils/user';

import type UserModel from '@typings/database/models/servers/user';

const WRAPPER_HEIGHT = 40;

const getStyleSheet = makeStyleSheetFromTheme((theme: Theme) => {
return {
wrapper: {
height: WRAPPER_HEIGHT,
},
container: {
backgroundColor: changeOpacity(theme.centerChannelColor, 0.08),
paddingVertical: 12,
paddingBottom: 20,
paddingHorizontal: 16,
color: changeOpacity(theme.centerChannelColor, 0.72),
display: 'flex',
flexDirection: 'row',
alignItems: 'center',
gap: 12,
position: 'absolute',
top: 0,
width: '100%',
},
text: {
color: changeOpacity(theme.centerChannelColor, 0.75),
fontSize: 14,
},
link: {
color: theme.linkColor,
fontSize: 14,
},
};
});

type Props = {
currentUser?: UserModel;
isMilitaryTime: boolean;
isThread?: boolean;
scheduledPostCount?: number;
}

export function ScheduledPostIndicator({currentUser, isMilitaryTime, isThread, scheduledPostCount = 0}: Props) {
const theme = useTheme();
const styles = getStyleSheet(theme);

let scheduledPostText: React.ReactNode;

if (scheduledPostCount === 0) {
return null;
} else if (scheduledPostCount === 1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Why not?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// eslint-disable-next-line no-warning-comments
//TODO: remove this hardcoded value with actual value
const value = 1738611689000;

const dateTime = (
<FormattedTime
timezone={getUserTimezone(currentUser)}
isMilitaryTime={isMilitaryTime}
value={value}
testID='scheduled_post_indicator_single_time'
/>
);

scheduledPostText = (
<FormattedMessage
id='scheduled_post.channel_indicator.single'
defaultMessage='Message scheduled for {dateTime}.'
values={{
dateTime,
}}
/>
);
} else {
scheduledPostText = isThread ? (
<FormattedMessage
id='scheduled_post.channel_indicator.thread.multiple'
defaultMessage='{count} scheduled messages in thread.'
values={{
count: scheduledPostCount,
}}
/>
) : (
<FormattedMessage
id='scheduled_post.channel_indicator.multiple'
defaultMessage='{count} scheduled messages in channel.'
values={{
count: scheduledPostCount,
}}
/>
);
}

return (
<View
className='ScheduledPostIndicator'
style={styles.wrapper}
>
<View style={styles.container}>
<CompassIcon
color={changeOpacity(theme.centerChannelColor, 0.6)}
name='clock-send-outline'
size={18}
/>
<Text style={styles.text}>
{scheduledPostText}
{' '}
<Text style={styles.link}>
<FormattedMessage
id='scheduled_post.channel_indicator.link_to_scheduled_posts.text'
defaultMessage='See all.'
/>
</Text>
Comment on lines +120 to +125
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

</Text>
</View>
</View>
);
}
4 changes: 4 additions & 0 deletions app/screens/channel/channel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {storeLastViewedChannelIdAndServer, removeLastViewedChannelIdAndServer} f
import FloatingCallContainer from '@calls/components/floating_call_container';
import FreezeScreen from '@components/freeze_screen';
import PostDraft from '@components/post_draft';
import ScheduledPostIndicator from '@components/scheduled_post_indicator';
import {ExtraKeyboardProvider} from '@context/extra_keyboard';
import useAndroidHardwareBackHandler from '@hooks/android_back_handler';
import {useChannelSwitch} from '@hooks/channel_switch';
Expand Down Expand Up @@ -149,6 +150,9 @@ const Channel = ({
nativeID={channelId}
/>
</View>
{/* eslint-disable-next-line no-warning-comments */}
{/*TODO: This count is hardcoded but will be removed during integration work*/}
<ScheduledPostIndicator scheduledPostCount={10}/>
<PostDraft
channelId={channelId}
testID='channel.post_draft'
Expand Down
4 changes: 4 additions & 0 deletions assets/base/i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -989,6 +989,10 @@
"saved_messages.empty.title": "No saved messages yet",
"scheduled_post_options.confirm_button.processing.text": "Scheduling",
"scheduled_post_options.confirm_button.text": "Schedule Draft",
"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}.",
Comment on lines +992 to +994
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

"scheduled_post.channel_indicator.thread.multiple": "{count} scheduled messages in thread.",
"scheduled_post.picker.custom": "Custom Time",
"scheduled_post.picker.monday": "Monday at {9amTime}",
"scheduled_post.picker.next_monday": "Next Monday at {9amTime}",
Expand Down
Loading
Loading