This repository has been archived by the owner on Mar 13, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
MM-22757 Fix for date toast placement in web mobile view #5090
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
f14abbc
MM-22757 Fix for date toast placement in web mobile view
sudheerDev fb14225
Rename func
sudheerDev d5537f8
Change to dispatching an action on change of toast
sudheerDev 399c36d
* Use className utility
sudheerDev aeeb399
Merge branch 'master' into MM-22757
sudheerDev c946840
Fix for lint
sudheerDev dfe3d73
Fix tests after master merge
sudheerDev 7512c38
Change default value in reducer
sudheerDev File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
20 changes: 20 additions & 0 deletions
20
components/post_view/floating_timestamp/__snapshots__/floating_timestamp.test.jsx.snap
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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> | ||
`; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
23 changes: 23 additions & 0 deletions
23
components/post_view/floating_timestamp/floating_timestamp.test.jsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 = { | ||
|
@@ -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); | ||
} | ||
} | ||
|
||
|
@@ -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), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1/5 can be simplified to !(atLatestPost && atBottom) There was a problem hiding this comment. Choose a reason for hiding this commentThe 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()} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.