Skip to content

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

Merged
mergify[bot] merged 2 commits intobox:masterfrom
mingzexiao6:add-resin-2
Jul 6, 2020
Merged

chore(resin): Add resin tags to mentions list#536
mergify[bot] merged 2 commits intobox:masterfrom
mingzexiao6:add-resin-2

Conversation

@mingzexiao6
Copy link
Contributor

No description provided.

@mingzexiao6 mingzexiao6 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?

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.

@mingzexiao6
Copy link
Contributor Author

mingzexiao6 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
@mingzexiao6 mingzexiao6 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