-
Notifications
You must be signed in to change notification settings - Fork 305
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
feat(boxai-sidebar): Empty Sidebar for Box AI #3668
feat(boxai-sidebar): Empty Sidebar for Box AI #3668
Conversation
test('should not fetch the file data if the id has not changed', () => { | ||
const wrapper = getWrapper({ fileId: '123' }); | ||
const instance = wrapper.instance(); | ||
const newProps = { fileId: '123' }; | ||
|
||
instance.fetchFile = jest.fn(); | ||
instance.setState({ view: 'activityFeed' }); | ||
instance.setState = jest.fn(); | ||
|
||
instance.componentDidUpdate(newProps); | ||
|
||
expect(instance.fetchFile).not.toBeCalled(); | ||
expect(instance.setState).not.toBeCalled(); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't that an exact copy of the above test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unresolving this comment as I don't understand why do we need this duplicated test?
5948f9b
to
002762c
Compare
The base branch was changed.
002762c
to
7bfea1f
Compare
src/elements/content-sidebar/stories/tests/BoxAISidebar-visual.stories.tsx
Outdated
Show resolved
Hide resolved
@greg-in-a-box could you please take another look? |
a42990f
to
bc720b9
Compare
test('should not fetch the file data if the id has not changed', () => { | ||
const wrapper = getWrapper({ fileId: '123' }); | ||
const instance = wrapper.instance(); | ||
const newProps = { fileId: '123' }; | ||
|
||
instance.fetchFile = jest.fn(); | ||
instance.setState({ view: 'activityFeed' }); | ||
instance.setState = jest.fn(); | ||
|
||
instance.componentDidUpdate(newProps); | ||
|
||
expect(instance.fetchFile).not.toBeCalled(); | ||
expect(instance.setState).not.toBeCalled(); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unresolving this comment as I don't understand why do we need this duplicated test?
For now PR needs SidebarPanels.test.js to be fixed, to be reviewed fully
Description
Added empty sidebar for Box AI, so the next features could be added to this empty container in next PRs.
Screenshots