Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

MM-22757 Fix for date toast placement in web mobile view #5090

Merged
merged 8 commits into from
Mar 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions actions/views/channel.js
Original file line number Diff line number Diff line change
Expand Up @@ -327,3 +327,12 @@ export function markChannelAsReadOnFocus(channelId) {
dispatch(markChannelAsRead(channelId));
};
}

export function updateToastStatus(status) {
return (dispatch) => {
dispatch({
type: ActionTypes.UPDATE_TOAST_STATUS,
data: status,
});
};
}
11 changes: 11 additions & 0 deletions actions/views/channel.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -519,4 +519,15 @@ describe('channel view actions', () => {
expect(markChannelAsRead).not.toHaveBeenCalled();
});
});

describe('updateToastStatus', () => {
test('should disptach updateToastStatus action with the true as argument', async () => {
await store.dispatch(Actions.updateToastStatus(true));

expect(store.getActions()).toEqual([{
data: true,
type: 'UPDATE_TOAST_STATUS'
Comment on lines +528 to +529
Copy link
Contributor

Choose a reason for hiding this comment

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

slightly concerned about the naming here: what should someone think of "update_toast_status=true"? maybe we should rename into something more descriptive?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i am 0/5 on this but this is not something that people stumble up on when reading code IMO.

}]);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`components/post_view/FloatingTimestamp should match snapshot 1`] = `
<div
className="post-list__timestamp scrolling toastAdjustment"
data-testid="floatingTimestamp"
>
<div>
<span>
<injectIntl(RecentDate)
day="2-digit"
month="short"
value={1234}
weekday="short"
year="numeric"
/>
</span>
</div>
</div>
`;
27 changes: 11 additions & 16 deletions components/post_view/floating_timestamp/floating_timestamp.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import PropTypes from 'prop-types';
import React from 'react';
import classNames from 'classnames';

import RecentDate from 'components/recent_date';

Expand All @@ -14,42 +15,36 @@ export default class FloatingTimestamp extends React.PureComponent {
PropTypes.instanceOf(Date),
PropTypes.number,
]).isRequired,
toastPresent: PropTypes.bool,
isRhsPost: PropTypes.bool,
stylesOverride: PropTypes.object,
}

render() {
if (!this.props.isMobile) {
return null;
}
const {isMobile, createAt, isScrolling, isRhsPost, toastPresent} = this.props;

if (this.props.createAt === 0) {
if (!isMobile || createAt === 0) {
return null;
}

const dateString = (
<RecentDate
value={this.props.createAt}
value={createAt}
weekday='short'
day='2-digit'
month='short'
year='numeric'
/>
);

let className = 'post-list__timestamp';
if (this.props.isScrolling) {
className += ' scrolling';
}

if (this.props.isRhsPost) {
className += ' rhs';
}
const classes = classNames('post-list__timestamp', {
scrolling: isScrolling,
rhs: isRhsPost,
toastAdjustment: toastPresent,
});

return (
<div
className={className}
style={this.props.stylesOverride}
className={classes}
data-testid='floatingTimestamp'
>
<div>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

import React from 'react';
import {shallow} from 'enzyme';

import FloatingTimestamp from './floating_timestamp.jsx';

describe('components/post_view/FloatingTimestamp', () => {
const baseProps = {
isScrolling: true,
isMobile: true,
createAt: 1234,
toastPresent: true,
isRhsPost: false,
};

test('should match snapshot', () => {
const wrapper = shallow(<FloatingTimestamp {...baseProps}/>);
expect(wrapper).toMatchSnapshot();
expect(wrapper.hasClass('toastAdjustment')).toBe(true);
});
});
4 changes: 4 additions & 0 deletions components/post_view/floating_timestamp/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {connect} from 'react-redux';
import {getPost} from 'mattermost-redux/selectors/entities/posts';
import * as PostListUtils from 'mattermost-redux/utils/post_list';

import {getToastStatus} from 'selectors/views/channel';

import FloatingTimestamp from './floating_timestamp';

function mapStateToProps(state, ownProps) {
Expand All @@ -18,8 +20,10 @@ function mapStateToProps(state, ownProps) {

const post = getPost(state, postId);

const toastPresent = getToastStatus(state);
return {
createAt: post ? post.create_at : 0,
toastPresent,
};
}

Expand Down
13 changes: 12 additions & 1 deletion components/toast_wrapper/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@
// See LICENSE.txt for license information.

import {connect} from 'react-redux';
import {bindActionCreators} from 'redux';
import {createSelector} from 'reselect';
import {Posts} from 'mattermost-redux/constants';
import {getAllPosts, getPostIdsInChannel} from 'mattermost-redux/selectors/entities/posts';
import {getCurrentUserId} from 'mattermost-redux/selectors/entities/users';
import {makePreparePostIdsForPostList} from 'mattermost-redux/utils/post_list';
import {countCurrentChannelUnreadMessages, isManuallyUnread} from 'mattermost-redux/selectors/entities/channels';

import {updateToastStatus} from 'actions/views/channel';

import ToastWrapper from './toast_wrapper.jsx';

export function makeCountUnreadsBelow() {
Expand Down Expand Up @@ -67,4 +70,12 @@ function makeMapStateToProps() {
};
}

export default connect(makeMapStateToProps)(ToastWrapper);
function mapDispatchToProps(dispatch) {
return {
actions: bindActionCreators({
updateToastStatus,
}, dispatch),
};
}

export default connect(makeMapStateToProps, mapDispatchToProps)(ToastWrapper);
62 changes: 47 additions & 15 deletions components/toast_wrapper/toast_wrapper.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ class ToastWrapper extends React.PureComponent {
scrollToNewMessage: PropTypes.func,
scrollToLatestMessages: PropTypes.func,
updateLastViewedBottomAt: PropTypes.func,
actions: PropTypes.shape({

/**
* Action creator to update toast status
*/
updateToastStatus: PropTypes.func.isRequired,
}).isRequired,
};

static defaultProps = {
Expand Down Expand Up @@ -107,25 +114,47 @@ class ToastWrapper extends React.PureComponent {

componentDidMount() {
this.mounted = true;
const {showUnreadToast, showNewMessagesToast, showMessageHistoryToast} = this.state;
const toastPresent = Boolean(showUnreadToast || showNewMessagesToast || showMessageHistoryToast);
document.addEventListener('keydown', this.handleShortcut);
this.props.actions.updateToastStatus(toastPresent);
}

componentDidUpdate(prevProps) {
if (!prevProps.atBottom && this.props.atBottom && this.props.atLatestPost) {
componentDidUpdate(prevProps, prevState) {
const {showUnreadToast, showNewMessagesToast, showMessageHistoryToast} = this.state;
const {
atBottom,
atLatestPost,
postListIds,
lastViewedBottom,
updateNewMessagesAtInChannel,
actions
} = this.props;

if (!prevProps.atBottom && atBottom && atLatestPost) {
this.hideNewMessagesToast(false);
this.hideUnreadToast();
this.hideArchiveToast();
}

const prevPostsCount = prevProps.postListIds.length;
const presentPostsCount = this.props.postListIds.length;
const postsAddedAtBottom = presentPostsCount !== prevPostsCount && this.props.postListIds[0] !== prevProps.postListIds[0];
const notBottomWithLatestPosts = !this.props.atBottom && this.props.atLatestPost && presentPostsCount > 0;
const presentPostsCount = postListIds.length;
const postsAddedAtBottom = presentPostsCount !== prevPostsCount && postListIds[0] !== prevProps.postListIds[0];
const notBottomWithLatestPosts = !atBottom && atLatestPost && presentPostsCount > 0;

//Marking existing messages as read based on last time user reached to the bottom
//This moves the new message indicator to the latest posts and keeping in sync with the toast count
if (postsAddedAtBottom && notBottomWithLatestPosts && !this.state.showUnreadToast) {
this.props.updateNewMessagesAtInChannel(this.props.lastViewedBottom);
if (postsAddedAtBottom && notBottomWithLatestPosts && !showUnreadToast) {
updateNewMessagesAtInChannel(lastViewedBottom);
}

const toastStateChanged = prevState.showUnreadToast !== showUnreadToast ||
prevState.showNewMessagesToast !== showNewMessagesToast ||
prevState.showMessageHistoryToast !== showMessageHistoryToast;

if (toastStateChanged) {
const toastPresent = Boolean(showUnreadToast || showNewMessagesToast || showMessageHistoryToast);
actions.updateToastStatus(toastPresent);
}
}

Expand Down Expand Up @@ -232,45 +261,48 @@ class ToastWrapper extends React.PureComponent {
}

render() {
const {atLatestPost, atBottom, width, lastViewedAt} = this.props;
const {showUnreadToast, showNewMessagesToast, showMessageHistoryToast, unreadCount} = this.state;

let unreadToastProps = {
show: false,
width: this.props.width,
width,
};

const archiveToastProps = {
show: Boolean(this.state.showMessageHistoryToast),
width: this.props.width,
show: Boolean(showMessageHistoryToast),
width,
onDismiss: this.hideArchiveToast,
onClick: this.scrollToLatestMessages,
onClickMessage: Utils.localizeMessage('postlist.toast.scrollToBottom', 'Jump to recents'),
showActions: true,
extraClasses: 'toast__history',
};

if (this.state.showUnreadToast && this.state.unreadCount > 0) {
if (showUnreadToast && unreadCount > 0) {
unreadToastProps = {
...unreadToastProps,
onDismiss: this.hideUnreadToast,
onClick: this.scrollToLatestMessages,
onClickMessage: Utils.localizeMessage('postlist.toast.scrollToBottom', 'Jump to recents'),
show: true,
showActions: !this.props.atLatestPost || (this.props.atLatestPost && !this.props.atBottom),
showActions: !atLatestPost || (atLatestPost && !atBottom),
Copy link
Contributor

Choose a reason for hiding this comment

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

1/5 can be simplified to !(atLatestPost && atBottom)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aah, Nice. i cannot wrap my head around conditions usually. Will change

};
} else if (this.state.showNewMessagesToast) {
} else if (showNewMessagesToast) {
unreadToastProps = {
...unreadToastProps,
onDismiss: this.hideNewMessagesToast,
onClick: this.scrollToNewMessage,
onClickMessage: Utils.localizeMessage('postlist.toast.scrollToLatest', 'Jump to new messages'),
show: true,
showActions: !this.props.atLatestPost || (this.props.atLatestPost && !this.props.atBottom),
showActions: !atLatestPost || (atLatestPost && !atBottom),
};
}

return (
<React.Fragment>
<Toast {...unreadToastProps}>
{this.newMessagesToastText(this.state.unreadCount, this.props.lastViewedAt)}
{this.newMessagesToastText(unreadCount, lastViewedAt)}
</Toast>
<Toast {...archiveToastProps}>
{this.archiveToastText()}
Expand Down
19 changes: 19 additions & 0 deletions components/toast_wrapper/toast_wrapper.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ describe('components/ToastWrapper', () => {
scrollToLatestMessages: jest.fn(),
updateLastViewedBottomAt: jest.fn(),
lastViewedAt: 12344,
actions: {
updateToastStatus: jest.fn(),
}
};

describe('unread count logic', () => {
Expand Down Expand Up @@ -387,5 +390,21 @@ describe('components/ToastWrapper', () => {
wrapper.setProps({postListIds: baseProps.postListIds});
expect(wrapper.state('showNewMessagesToast')).toBe(false);
});

test('Should call updateToastStatus on toasts state change', () => {
const props = {
...baseProps,
unreadCountInChannel: 10,
newRecentMessagesCount: 5
};
const updateToastStatus = baseProps.actions.updateToastStatus;

const wrapper = shallowWithIntl(<ToastWrapper {...props}/>);
expect(wrapper.state('showUnreadToast')).toBe(true);
expect(updateToastStatus).toHaveBeenCalledWith(true);
wrapper.setProps({atBottom: true, atLatestPost: true});
expect(updateToastStatus).toHaveBeenCalledTimes(2);
expect(updateToastStatus).toHaveBeenCalledWith(false);
});
});
});
12 changes: 12 additions & 0 deletions reducers/views/channel.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,17 @@ function lastGetPosts(state = {}, action) {
}
}

function toastStatus(state = false, action) {
switch (action.type) {
case ActionTypes.SELECT_CHANNEL_WITH_MEMBER:
return false;
case ActionTypes.UPDATE_TOAST_STATUS:
return action.data;
default:
return state;
}
}

export default combineReducers({
postVisibility,
lastChannelViewTime,
Expand All @@ -147,4 +158,5 @@ export default combineReducers({
mobileView,
keepChannelIdAsUnread,
lastGetPosts,
toastStatus,
});
4 changes: 4 additions & 0 deletions sass/layout/_post.scss
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,10 @@
top: 98px;
}

&.toastAdjustment {
top: 50px;
}

> div {
@include border-radius(3px);
@include font-smoothing(initial);
Expand Down
1 change: 1 addition & 0 deletions selectors/views/channel.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
// See LICENSE.txt for license information.

export const getLastPostsApiTimeForChannel = (state, channelId) => state.views.channel.lastGetPosts[channelId];
export const getToastStatus = (state) => state.views.channel.toastStatus;
1 change: 1 addition & 0 deletions utils/constants.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ export const ActionTypes = keyMirror({
POST_UNREAD_SUCCESS: null,

SET_UNREAD_FILTER_ENABLED: null,
UPDATE_TOAST_STATUS: null,
});

export const PostRequestTypes = keyMirror({
Expand Down