MM-16152: always focus RHS on SELECT_POST

actions/post_actions.jsx

@@ -243,6 +243,7 @@ export function deleteAndRemovePost(post) {
  type: ActionTypes.SELECT_POST,
  postId: '',
  channelId: '',
+ timestamp: 0,
  });
  }
 

actions/views/rhs.js

@@ -50,6 +50,7 @@ export function selectPostFromRightHandSideSearch(post) {
  postId: postRootId,
  channelId: post.channel_id,
  previousRhsState: getRhsState(getState()),
+ timestamp: Date.now(),
  });
  };
 }
@@ -219,6 +220,7 @@ export function closeRightHandSide() {
  type: ActionTypes.SELECT_POST,
  postId: '',
  channelId: '',
+ timestamp: 0,
  },
  ]));
  };
@@ -250,7 +252,12 @@ export function toggleRhsExpanded() {
 }
 
 export function selectPost(post) {
- return {type: ActionTypes.SELECT_POST, postId: post.root_id || post.id, channelId: post.channel_id};
+ return {
+ type: ActionTypes.SELECT_POST,
+ postId: post.root_id || post.id,
+ channelId: post.channel_id,
+ timestamp: Date.now(),
+ };
 }
 
 export function selectPostCard(post) {

actions/views/rhs.test.js

@@ -35,6 +35,10 @@ const currentUserId = 'user123';
 const UserSelectors = require('mattermost-redux/selectors/entities/users');
 UserSelectors.getCurrentUserMentionKeys = jest.fn(() => [{key: '@here'}, {key: '@mattermost'}, {key: '@channel'}, {key: '@all'}]);
 
+// Mock Date.now() to return a constant value.
+const POST_CREATED_TIME = Date.now();
+global.Date.now = jest.fn(() => POST_CREATED_TIME);
+
 jest.mock('mattermost-redux/actions/posts', () => ({
  getPostThread: (...args) => ({type: 'MOCK_GET_POST_THREAD', args}),
  getProfilesAndStatusesForPosts: (...args) => ({type: 'MOCK_GET_PROFILES_AND_STATUSES_FOR_POSTS', args}),
@@ -120,26 +124,29 @@ describe('rhs view actions', () => {
  expect(store.getActions()[0]).toEqual(compareStore.getActions()[0]);
  });
 
- test(`it dispatches ${ActionTypes.SELECT_POST} correctly`, async () => {
- store = mockStore({
- ...initialState,
- views: {
- rhs: {
- rhsState: RHSStates.FLAG,
+ describe(`it dispatches ${ActionTypes.SELECT_POST} correctly`, () => {
+ it('with mocked date', async () => {
+ store = mockStore({
+ ...initialState,
+ views: {
+ rhs: {
+ rhsState: RHSStates.FLAG,
+ },
  },
- },
- });
+ });
 
- await store.dispatch(selectPostFromRightHandSideSearch(post));
+  await store.dispatch(selectPostFromRightHandSideSearch(post));
 
- const action = {
- type: ActionTypes.SELECT_POST,
- postId: post.root_id,
- channelId: post.channel_id,
- previousRhsState: RHSStates.FLAG,
- };
+ const action = {
+ type: ActionTypes.SELECT_POST,
+ postId: post.root_id,
+ channelId: post.channel_id,
+ previousRhsState: RHSStates.FLAG,
+ timestamp: POST_CREATED_TIME,
+ };
 
- expect(store.getActions()[1]).toEqual(action);
+ expect(store.getActions()[1]).toEqual(action);
+ });
  });
  });
 
@@ -413,6 +420,7 @@ describe('rhs view actions', () => {
  type: ActionTypes.SELECT_POST,
  postId: '',
  channelId: '',
+ timestamp: 0,
  },
  ]));
 

components/create_comment/create_comment.jsx

@@ -180,6 +180,11 @@ export default class CreateComment extends React.PureComponent {
  * To check if the timezones are enable on the server.
  */
  isTimezoneEnabled: PropTypes.bool.isRequired,
+
+ /**
+ * The last time, if any, when the selected post changed. Will be 0 if no post selected.
+ */
+ selectedPostFocussedAt: PropTypes.number.isRequired,
  }
 
  static contextTypes = {
@@ -251,7 +256,7 @@ export default class CreateComment extends React.PureComponent {
  this.scrollToBottom();
  }
 
- if (prevProps.rootId !== this.props.rootId) {
+ if (prevProps.rootId !== this.props.rootId || this.props.selectedPostFocussedAt > this.lastBlurAt) {
  this.focusTextbox();
  }
 

components/create_comment/create_comment.test.jsx

@@ -48,6 +48,7 @@ describe('components/CreateComment', () => {
  badConnection: false,
  getChannelTimezones: jest.fn(() => Promise.resolve([])),
  isTimezoneEnabled: false,
+ selectedPostFocussedAt: 0,
  };
 
  test('should match snapshot, empty comment', () => {
@@ -343,24 +344,79 @@ describe('components/CreateComment', () => {
  expect(showPostDeletedModal).toHaveBeenCalled();
  });
 
- /* Removed due setProps not actually setting the props
- * test('calls focusTextbox when rootId changes', () => {
+ describe('focusTextbox', () => {
  const draft = {
  message: 'Test message',
  uploadsInProgress: [1, 2, 3],
  fileInfos: [{id: '1', name: 'aaa', create_at: 100}, {id: '2', name: 'bbb', create_at: 200}],
  };
- const props = {...baseProps, draft};
 
- const wrapper = shallowWithIntl(
- <CreateComment {...props}/>
- );
+ it('is called when rootId changes', () => {
+ const props = {...baseProps, draft};
+ const wrapper = shallowWithIntl(
+ <CreateComment {...props}/>
+ );
 
- const focusTextbox = jest.fn();
- wrapper.instance().focusTextbox = focusTextbox;
- wrapper.setProps({rootId: 'testid123'});
- expect(focusTextbox).toHaveBeenCalled();
- });*/
+ const focusTextbox = jest.fn();
+ wrapper.instance().focusTextbox = focusTextbox;
+
+ const newProps = {
+ ...props,
+ rootId: 'testid123',
+ };
+
+ // Note that setProps doesn't actually trigger componentDidUpdate
+ wrapper.setProps(newProps);
+ wrapper.instance().componentDidUpdate(props, newProps);
+ expect(focusTextbox).toHaveBeenCalled();
+ });
+
+ it('is not called when rootId does not change', () => {
+ const props = {...baseProps, draft};
+ const wrapper = shallowWithIntl(
+ <CreateComment {...props}/>
+ );
+
+ const focusTextbox = jest.fn();
+ wrapper.instance().focusTextbox = focusTextbox;
+
+ // Note that setProps doesn't actually trigger componentDidUpdate
+ wrapper.setProps(props);
+ wrapper.instance().componentDidUpdate(props, props);
+ expect(focusTextbox).not.toHaveBeenCalled();
+ });
+
+ it('is called when rootId does not change but the selectPostFocussedAt occurred after the last blur', () => {
+ const props = {...baseProps, draft, selectedPostFocussedAt: 1000};
+ const wrapper = shallowWithIntl(
+ <CreateComment {...props}/>
+ );
+
+ const focusTextbox = jest.fn();
+ wrapper.instance().focusTextbox = focusTextbox;
+
+ // Note that setProps doesn't actually trigger componentDidUpdate
+ wrapper.setProps(props);
+ wrapper.instance().componentDidUpdate(props, props);
+ expect(focusTextbox).toHaveBeenCalled();
+ });
+
+ it('is not called when rootId does not change and the selectPostFocussedAt occurred before the last blur', () => {
+ const props = {...baseProps, draft, selectedPostFocussedAt: 1000};
+ const wrapper = shallowWithIntl(
+ <CreateComment {...props}/>
+ );
+
+ const focusTextbox = jest.fn();
+ wrapper.instance().focusTextbox = focusTextbox;
+ wrapper.instance().handleBlur();
+
+ // Note that setProps doesn't actually trigger componentDidUpdate
+ wrapper.setProps(props);
+ wrapper.instance().componentDidUpdate(props, props);
+ expect(focusTextbox).not.toHaveBeenCalled();
+ });
+ });
 
  test('handleChange should update comment draft correctly', () => {
  const onUpdateCommentDraft = jest.fn();

components/create_comment/index.js

@@ -24,7 +24,7 @@ import {
  makeOnSubmit,
  makeOnEditLatestPost,
 } from 'actions/views/create_comment';
-import {getPostDraft, getIsRhsExpanded} from 'selectors/rhs';
+import {getPostDraft, getIsRhsExpanded, getSelectedPostFocussedAt} from 'selectors/rhs';
 
 import CreateComment from './create_comment.jsx';
 
@@ -66,6 +66,7 @@ function makeMapStateToProps() {
  rhsExpanded: getIsRhsExpanded(state),
  badConnection,
  isTimezoneEnabled,
+ selectedPostFocussedAt: getSelectedPostFocussedAt(state),
  };
  };
 }

reducers/views/rhs.js

@@ -28,6 +28,17 @@ function selectedPostId(state = '', action) {
  }
 }
 
+// selectedPostFocussedAt keeps track of the last time a post was selected, whether or not it
+// is currently selected.
+function selectedPostFocussedAt(state = 0, action) {
+ switch (action.type) {
+ case ActionTypes.SELECT_POST:
+ return action.timestamp || 0;
+ default:
+ return state;
+ }
+}
+
 function selectedPostCardId(state = '', action) {
  switch (action.type) {
  case ActionTypes.SELECT_POST_CARD:
@@ -217,6 +228,7 @@ function isMenuOpen(state = false, action) {
 
 export default combineReducers({
  selectedPostId,
+ selectedPostFocussedAt,
  selectedPostCardId,
  selectedChannelId,
  previousRhsState,

reducers/views/rhs.test.js

@@ -9,6 +9,7 @@ import {ActionTypes, RHSStates} from 'utils/constants.jsx';
 describe('Reducers.RHS', () => {
  const initialState = {
  selectedPostId: '',
+ selectedPostFocussedAt: 0,
  selectedPostCardId: '',
  selectedChannelId: '',
  previousRhsState: null,
@@ -157,12 +158,14 @@ describe('Reducers.RHS', () => {
  type: ActionTypes.SELECT_POST,
  postId: '123',
  channelId: '321',
+ timestamp: 1234,
  }
  );
 
  expect(nextState1).toEqual({
  ...initialState,
  selectedPostId: '123',
+ selectedPostFocussedAt: 1234,
  selectedChannelId: '321',
  isSidebarOpen: true,
  });
@@ -175,12 +178,14 @@ describe('Reducers.RHS', () => {
  postId: '123',
  channelId: '321',
  previousRhsState: RHSStates.SEARCH,
+ timestamp: 4567,
  }
  );
 
  expect(nextState2).toEqual({
  ...initialState,
  selectedPostId: '123',
+ selectedPostFocussedAt: 4567,
  selectedChannelId: '321',
  previousRhsState: RHSStates.SEARCH,
  isSidebarOpen: true,
@@ -195,12 +200,14 @@ describe('Reducers.RHS', () => {
  postId: '123',
  channelId: '321',
  previousRhsState: RHSStates.FLAG,
+ timestamp: 0,
  }
  );
 
  expect(nextState3).toEqual({
  ...initialState,
  selectedPostId: '123',
+ selectedPostFocussedAt: 0,
  selectedChannelId: '321',
  previousRhsState: RHSStates.FLAG,
  isSidebarOpen: true,

selectors/rhs.jsx

@@ -12,6 +12,10 @@ export function getSelectedPostId(state) {
  return state.views.rhs.selectedPostId;
 }
 
+export function getSelectedPostFocussedAt(state) {
+ return state.views.rhs.selectedPostFocussedAt;
+}
+
 export function getSelectedPostCardId(state) {
  return state.views.rhs.selectedPostCardId;
 }

selectors/rhs.test.js

@@ -6,6 +6,18 @@ import assert from 'assert';
 import * as Selectors from 'selectors/rhs.jsx';
 
 describe('Selectors.Rhs', () => {
+ describe('should return the last time a post was selected', () => {
+ [0, 1000000, 2000000].forEach((expected) => {
+ it(`when open is ${expected}`, () => {
+ const state = {views: {rhs: {
+ selectedPostFocussedAt: expected,
+ }}};
+
+ assert.deepEqual(expected, Selectors.getSelectedPostFocussedAt(state));
+ });
+ });
+ });
+
  describe('should return the open state of the sidebar', () => {
  [true, false].forEach((expected) => {
  it(`when open is ${expected}`, () => {