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

chore(resin): Add resin tags to mentions list #536

Merged
merged 2 commits into from
Jul 6, 2020

Conversation

mxiao6
Copy link
Contributor

@mxiao6 mxiao6 commented Jul 2, 2020

No description provided.

@mxiao6 mxiao6 requested a review from a team as a code owner July 2, 2020 04:38
onSelect,
...rest
}: Props<T>): JSX.Element => {
const ItemList = <T extends { id: string }>(props: Props<T>): JSX.Element => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file is meant to be a generic component, right? Should the resin tags live on a decorated version of ItemRow, instead?

src/components/ReplyField/ReplyFieldContainer.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@ConradJChan ConradJChan left a comment

Choose a reason for hiding this comment

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

RE: #536 (comment), seems like moving the resin tags to ItemRow is still too specific toward at-mention

let reduxSpy: jest.SpyInstance;

beforeEach(() => {
reduxSpy = jest.spyOn(ReactRedux, 'useSelector').mockImplementation(() => true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this mockImplementation needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah it is needed since we use shallow above instead of mount.

@mxiao6
Copy link
Contributor Author

mxiao6 commented Jul 6, 2020

RE: #536 (comment), seems like moving the resin tags to ItemRow is still too specific toward at-mention

ItemRow is not a generic component since it has a required prop item which is specific to Collaborator. So I think it's ok to add more specific things to ItemRow.

@mergify mergify bot merged commit 38c1fbc into box:master Jul 6, 2020
@mxiao6 mxiao6 deleted the add-resin-2 branch August 12, 2020 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants