chore(content-explorer): Migrate ContentExplorer #3870
Conversation
greg-in-a-box
force-pushed
the
content-explorer
branch
2 times, most recently
from
ecbd64e to
e9419ae
Compare
greg-in-a-box
force-pushed
the
content-explorer
branch
6 times, most recently
from
0aef079 to
ffe382a
Compare
greg-in-a-box
force-pushed
the
content-explorer
branch
from
ffe382a to
ab5cf70
Compare
| token: Token, | ||
| uploadHost: string, | ||
| }; | ||
| export interface ContentExplorerProps { |
There was a problem hiding this comment.
I see that many props are changed to optional. Is the intention that only the token prop is needed? Can we verify that this is the only prop required to get Explorer to run?
There was a problem hiding this comment.
thats what it seems like, only the rootFolderId and token is needed, thats pretty much whats in the basic storybook for contentexplorer,
There was a problem hiding this comment.
Do we need flow files anymore for this component? Once the root level element is migrated, what's the purpose of the flow files?
There was a problem hiding this comment.
we can remove it in a separate PR, otherwise this PR will be huge
There was a problem hiding this comment.
what do we use this file for? there are no types?
There was a problem hiding this comment.
came with the codemod
There was a problem hiding this comment.
I say we start removing the flow files for this element
| name: 'Jeremy Press', | ||
| login: 'jpress@boxdemo.com', |
There was a problem hiding this comment.
I don't think we should codify people's names in the repo. This is an ex-boxer
| @@ -42,36 +42,43 @@ export const openDeleteConfirmationDialog = { | |||
| play: async ({ canvasElement }) => { | |||
| const canvas = within(canvasElement); | |||
|
|
|||
| const moreOptionsButton = await canvas.findByRole('button', { name: 'More options' }); | |||
| await userEvent.click(moreOptionsButton); | |||
| const moreOptionsButton = await canvas.findAllByRole('button', { name: 'More options' }); | |||
There was a problem hiding this comment.
are there multiple buttons that show with the "More Options" button? if not why this change?
There was a problem hiding this comment.
there are multiple, have to indicator which one
greg-in-a-box
force-pushed
the
content-explorer
branch
2 times, most recently
from
8e2b746 to
7c88ceb
Compare
greg-in-a-box
force-pushed
the
content-explorer
branch
from
7c88ceb to
506fada
Compare
Updated the mocks
Updated the styleguide config to switch to a tsx
Updated ContentExplorer to use TS
Remove old Enzyme test
Added new RTL tests, increase coverage significantly but still room for improvement
Updated VRTs
Updated some missing types