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

[GH-3959] implement Option to "Create New Category" under the "Move To..." option in the sidebar #4057

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

cannalee90
Copy link
Contributor

@cannalee90 cannalee90 commented Oct 20, 2022

Summary

I just implement option to create new category under the option in the sidebar

please look through the last commit about moving board to newly created category

I'm not it is right way to do but I have no idea how to get category id that I just created

2022-10-21 01 11 12

Ticket Link

Fixes #3959

@mattermod
Copy link
Contributor

Hello @cannalee90,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

@cannalee90 cannalee90 changed the title [GH-3959] implement Option to "Create New Category" under the "Move To..." option in the sidebar (WIP) [GH-3959] implement Option to "Create New Category" under the "Move To..." option in the sidebar Oct 20, 2022
@cannalee90 cannalee90 marked this pull request as draft October 20, 2022 15:49
@cannalee90 cannalee90 marked this pull request as ready for review October 20, 2022 16:13
@cannalee90 cannalee90 changed the title (WIP) [GH-3959] implement Option to "Create New Category" under the "Move To..." option in the sidebar [GH-3959] implement Option to "Create New Category" under the "Move To..." option in the sidebar Oct 20, 2022
Copy link
Contributor

@jespino jespino left a comment

Choose a reason for hiding this comment

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

This is looking great, some guidenace around how to get properly the new created category id.

@cannalee90
Copy link
Contributor Author

@jespino I apply your review! thanks a lot

@cannalee90
Copy link
Contributor Author

I applied your review. @jespino and feel free to talk to me anything you bother you.

@harshilsharma63
Copy link
Member

@cannalee90 this is awesome! It works so well! Could you please add some unit tests for this? Everything else looks fantastic!

Copy link
Member

@harshilsharma63 harshilsharma63 left a comment

Choose a reason for hiding this comment

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

The code looks awesome! Could you add some unit tests?

Copy link
Contributor

@jespino jespino left a comment

Choose a reason for hiding this comment

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

A tiny detail, and as @harshilsharma63 suggested, would be great to have some unit tests for this. But for me the code is ready (apart from the little suggestion on parameter name).

Copy link
Contributor

@jespino jespino left a comment

Choose a reason for hiding this comment

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

Now Is ready for me, still waiting for the unit tests requested from @harshilsharma63

@cannalee90
Copy link
Contributor Author

Ill try my best!

@cannalee90
Copy link
Contributor Author

cannalee90 commented Oct 27, 2022

@harshilsharma63 @jespino

I'm trying to add some unit test in sidebarBoarItem.test.tsx but have problem updating snapshot for showing nested menu. Would you help me on this? Here is What I wrote

    test('should have create category item', () => {
        const mockStore = configureStore([])
        const store = mockStore({...state})

        const component = wrapIntl(
            <ReduxProvider store={store}>
                <Router history={history}>
                    <SidebarBoardItem
                        categoryBoards={categoryBoards1}
                        board={board}
                        allCategories={allCategoryBoards}
                        isActive={true}
                        showBoard={jest.fn()}
                        showView={jest.fn()}
                        onDeleteRequest={jest.fn()}
                    />
                </Router>
            </ReduxProvider>,
        )
        const {container} = render(component)
        const elementMenuWrapper = container.querySelector('.SidebarBoardItem div.MenuWrapper')
        expect(elementMenuWrapper).not.toBeNull()
        act(() => {
            userEvent.click(elementMenuWrapper!)
        })
        act(() => {
            const moveToElement = container.querySelector('#moveBlock')
            userEvent.hover(moveToElement!)
        })
        const createCategoryElem = container.querySelector('#createNewCategory')
        expect(createCategoryElem).not.toBeNull()
        expect(container).toMatchSnapshot()
    })

@cannalee90
Copy link
Contributor Author

Oh finally I made it! please review for the test @harshilsharma63

@sbishel
Copy link
Collaborator

sbishel commented Nov 2, 2022

@harshilsharma63 - Please review.

Copy link
Member

@harshilsharma63 harshilsharma63 left a comment

Choose a reason for hiding this comment

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

Awesome! LGTM 👍

@mattermod
Copy link
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

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.

Feature Idea: Option to "Create New Category" under the "Move To..." option in the sidebar
5 participants