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(content-uploader): Migrated ItemAction #3600

Merged
merged 1 commit into from
Aug 6, 2024

Conversation

greg-in-a-box
Copy link
Contributor

@greg-in-a-box greg-in-a-box commented Aug 2, 2024

  • Converted to typescript
  • Converted old .js to .js.flow
  • Converted to react-testing-library
  • Updated ItemList test to account for changes in ItemAction but did not convert to react-testing-library yet, will convert when ItemList is converted
  • Deleted snapshots
  • Replaced BUIE tooltip with blueprint tooltip - added TooltipProvider to the root of the ContentUploader - Also added it to the testing-library utility
  • Replaced BUIE icons for blueprint icons

@greg-in-a-box greg-in-a-box marked this pull request as ready for review August 2, 2024 22:12
@greg-in-a-box greg-in-a-box requested review from a team as code owners August 2, 2024 22:12
@@ -63,15 +63,15 @@ const ItemList = ({
cellRenderer={actionCell}
dataKey="status"
flexShrink={0}
width={onUpgradeCTAClick ? 100 : 25}
width={onUpgradeCTAClick ? 100 : 32}
Copy link
Contributor

Choose a reason for hiding this comment

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

can we store this in a constant

/>
{isResumableUploadsEnabled && (
<Column
className="bcu-item-list-action-column"
cellRenderer={removeCell}
dataKey="remove"
flexShrink={0}
width={25}
width={32}
Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking more of storing 100/32 as a constant so that it can be used in both places

Comment on lines +4 to +5
import { IntlProvider } from 'react-intl';
import { TooltipProvider } from '@box/blueprint-web';
Copy link
Contributor

Choose a reason for hiding this comment

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

How come this test isn't migrated, but ItemAction.test.tsx is? Did we decide to do this as a separate task?

Copy link
Contributor

@QingyuChai QingyuChai left a comment

Choose a reason for hiding this comment

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

lgtm! Thank you Greg!

@mergify mergify bot merged commit 54b4c83 into box:master Aug 6, 2024
6 checks passed
kajarosz pushed a commit to kajarosz/box-ui-elements that referenced this pull request Aug 21, 2024
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