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

Migrate post_list_row component to typescript #6593

Merged
merged 4 commits into from
Oct 3, 2020
Merged

Migrate post_list_row component to typescript #6593

merged 4 commits into from
Oct 3, 2020

Conversation

sowmiyamuthuraman
Copy link
Contributor

@hanzei hanzei added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester Hacktoberfest labels Oct 1, 2020
devinbinnie
devinbinnie suggested changes Oct 1, 2020
View reviewed changes
Copy link
Member

@devinbinnie devinbinnie left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @sowmiyamuthuraman.
I've added a couple comments for you to review.

components/post_view/post_list_row/index.js
@@ -7,7 +7,7 @@ import {bindActionCreators} from 'redux';
import {getShortcutReactToLastPostEmittedFrom} from 'selectors/emojis';
import {emitShortcutReactToLastPostFrom} from 'actions/post_actions.jsx';

import PostListRow from './post_list_row.jsx';
import PostListRow from './post_list_row';
Copy link
Member

Choose a reason for hiding this comment

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

This file should also be migrated to TypeScript.

components/post_view/post_list_row/post_list_row.tsx Outdated
previousListId?: string,
fullWidth?: boolean,
shouldHighlight?: boolean,
loadOlderPosts: () => any,
Copy link
Member

Choose a reason for hiding this comment

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

These shouldn't return type any. If we don't need the return type, use void instead.

components/post_view/post_list_row/post_list_row.tsx Outdated
isLastPost: PropTypes.bool,
* Function to set or unset emoji picker for last message
*/
emitShortcutReactToLastPostFrom: (location:string) => any
Copy link
Member

Choose a reason for hiding this comment

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

Same here.

devinbinnie
devinbinnie approved these changes Oct 2, 2020
View reviewed changes
Copy link
Member

@devinbinnie devinbinnie left a comment

Choose a reason for hiding this comment

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

Looking good now, I have some non-blocking comments but overall it's good.
Thanks @sowmiyamuthuraman!

components/post_view/post_list_row/index.ts Outdated

import {getShortcutReactToLastPostEmittedFrom} from 'selectors/emojis';
import {emitShortcutReactToLastPostFrom} from 'actions/post_actions.jsx';

import PostListRow from './post_list_row.jsx';
import {GlobalState} from '../../../types/store';
Copy link
Member

Choose a reason for hiding this comment

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

This can just be types/store, no need to use an explicit directory reference.

components/post_view/post_list_row/index.ts Outdated

function mapStateToProps(state, ownProps) {
function mapStateToProps(state:GlobalState, ownProps:Props) {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: spacing between : and GlobalState. Also between : and Props

components/post_view/post_list_row/index.ts Outdated
@@ -19,7 +31,7 @@ function mapStateToProps(state, ownProps) {
};
}

function mapDispatchToProps(dispatch) {
function mapDispatchToProps(dispatch:Dispatch<GenericAction>) {
Copy link
Member

Choose a reason for hiding this comment

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

Same nit as above

components/post_view/post_list_row/post_list_row.tsx Outdated
shortcutReactToLastPostEmittedFrom: PropTypes.string,
* Function to set or unset emoji picker for last message
*/
emitShortcutReactToLastPostFrom: (location:string) => void
Copy link
Member

Choose a reason for hiding this comment

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

Nit: spacing between : and string

components/post_view/post_list_row/post_list_row.tsx Outdated

blockShortcutReactToLastPostForNonMessages(listId) {
export default class PostListRow extends React.PureComponent<PostListRowProps> {
blockShortcutReactToLastPostForNonMessages(listId:string) {
Copy link
Member

Choose a reason for hiding this comment

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

Same nit as above

mickmister
mickmister approved these changes Oct 2, 2020
View reviewed changes
Copy link
Contributor

@mickmister mickmister left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @sowmiyamuthuraman!

@mickmister mickmister removed the 2: Dev Review Requires review by a core commiter label Oct 2, 2020
@saturninoabril saturninoabril added Setup Cloud Test Server Setup a test server using Mattermost Cloud and removed Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Oct 3, 2020
@mm-cloud-bot
Copy link

Test server destroyed

@saturninoabril
Copy link
Member

Thanks @sowmiyamuthuraman! Tested and passed. LGTM.

@saturninoabril saturninoabril added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Oct 3, 2020
@saturninoabril saturninoabril merged commit 8cf1be9 into mattermost:master Oct 3, 2020
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Oct 5, 2020
Tak-Iwamoto pushed a commit to Tak-Iwamoto/mattermost-webapp that referenced this pull request Oct 6, 2020
Merge branch 'master' of github.com:Tak-Iwamoto/mattermost-webapp int…
d6f9471 
…o MM-20462

* 'master' of github.com:Tak-Iwamoto/mattermost-webapp:
  [MM-22161] - Improve accessibility of channel switcher (mattermost#6199)
  [MM-20460] Migrate 'components/permissions_gates/channel_permission_gate' module and associated tests to TypeScript (mattermost#6601)
  [GH-15724] Remove usage of findDOMNode from Create Team screens (mattermost#6621)
  [MM-29302] Billing Summary Card - No Billing History case (mattermost#6639)
  /files should target=_blank (mattermost#6616)
  MM T134 visual verification of tooltips (mattermost#6604)
  MM-24812 Add e2e tests for adding reactions to existing reactions in archived channel posts (mattermost#6579)
  [MM-20490] Migrate 'components/post_view/combined_user_activity_post' module and associated tests to TypeScript (mattermost#6632)
  MM-24810 Add e2e for reactions for archived channel posts (mattermost#6562)
  [MM-28965] - Prevent link overlap in channel header (mattermost#6546)
  MM-28780: Migrate dotMenu string refs to functional ones (mattermost#6592)
  Cypress/E2E: Fix archived channel specs (mattermost#6617)
  Fixed cypress env for PostgreSQL connection (mattermost#6627)
  Migrate post_list_row component to typescript (mattermost#6593)
  Cypress/E2E: Fix managing bot accounts spec (mattermost#6614)
  Cypress/E2E: Stabilize profile popover spec (mattermost#6612)
  Migrated string refs in signup_email (mattermost#6602)
  Renaming diagnostics-to-telemetry (mattermost#5741)
jfrerich pushed a commit that referenced this pull request Oct 23, 2020
@sowmiyamuthuraman @jfrerich
Migrate post_list_row component to typescript (#6593)
61838e5 
* MM-20476: Migrate post_list_row component to typescript

* MM-20476: Use void instead of any in function return types

* MM-20476: Migrate post_list_row/index.js to typescript

* MM-20476: Fix lint issues
calebroseland pushed a commit that referenced this pull request Oct 27, 2020
@sowmiyamuthuraman @calebroseland
Migrate post_list_row component to typescript (#6593)
e41f71c 
* MM-20476: Migrate post_list_row component to typescript

* MM-20476: Use void instead of any in function return types

* MM-20476: Migrate post_list_row/index.js to typescript

* MM-20476: Fix lint issues
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mickmister mickmister mickmister approved these changes

@devinbinnie devinbinnie devinbinnie approved these changes

@saturninoabril saturninoabril Awaiting requested review from saturninoabril

Assignees
No one assigned
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation Hacktoberfest hacktoberfest-accepted
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Migrate 'components/post_view/post_list_row' module and associated tests to TypeScript
8 participants
@sowmiyamuthuraman @mm-cloud-bot @saturninoabril @mickmister @devinbinnie @jasonblais @hanzei @amyblais