-
Notifications
You must be signed in to change notification settings - Fork 336
refactor(content-picker): convert Content to TypeScript #3944
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
base: master
Are you sure you want to change the base?
refactor(content-picker): convert Content to TypeScript #3944
Conversation
- Renamed Content.js to Content.tsx - Converted Flow types to TypeScript interfaces - Improved type safety with proper function signatures - Removed Flow comments and simplified JSDoc Link to Devin run: https://app.devin.ai/sessions/848dfdad3fe447138f3a218f3058e660 Requested by: [email protected]
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
| import ItemList from './ItemList'; | ||
| import { VIEW_ERROR, VIEW_SELECTED } from '../../constants'; | ||
| import type { View, Collection } from '../../common/types/core'; | ||
| import { View, Collection } from '../../common/types/core'; |
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.
Can you sort this?
| import { View, Collection } from '../../common/types/core'; | |
| import { Collection, View } from '../../common/types/core'; |
| * | ||
| * @param {string} view the current view | ||
| * @param {Object} currentCollection the current collection | ||
| * @return {boolean} empty or not |
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.
Please revert this change.
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.
Devin please revert this change
- Sort imports alphabetically - Restore JSDoc comments
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.
don't merge this yet, as the JS component could be useful for testing. however, it looks good and doesn't require any more changes.
| /** | ||
| * @flow | ||
| * @file File picker header and list component | ||
| * @author Box | ||
| */ | ||
|
|
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.
Please delete this comment, you don't need the header comment at the top of the file
| @@ -68,24 +65,22 @@ const Content = ({ | |||
| <ProgressBar percent={currentCollection.percentLoaded} /> | |||
| )} | |||
| {isEmpty(view, currentCollection) ? ( | |||
| <EmptyView view={view} isLoading={currentCollection.percentLoaded !== 100} /> | |||
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.
why did you make this change?
| canSetShareAccess, | ||
| onItemSelect, | ||
| onItemClick, | ||
| onShareAccessChange, |
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.
please revert these changes
| onFocusChange={onFocusChange} | ||
| onShareAccessChange={onShareAccessChange} | ||
| onFocusChange={onFocusChange} | ||
| extensionsWhitelist={extensionsWhitelist} |
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.
please revert these changes
| */ | ||
|
|
||
| import * as React from 'react'; | ||
| // $FlowFixMe TypeScript file |
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.
revert this change
| view, | ||
| rootId, | ||
| isSmall, | ||
| rootElement, | ||
| focusedRow, | ||
| hasHitSelectionLimit, | ||
| selectableType, | ||
| currentCollection, | ||
| tableRef, | ||
| canSetShareAccess, | ||
| isSingleSelect, | ||
| onItemClick, | ||
| onItemSelect, | ||
| onShareAccessChange, | ||
| onFocusChange, | ||
| extensionsWhitelist, |
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.
can you sort this list alphabetically?
| view={view} | ||
| rootId={rootId} | ||
| isSmall={isSmall} | ||
| rootElement={rootElement} | ||
| focusedRow={focusedRow} | ||
| currentCollection={currentCollection} | ||
| tableRef={tableRef} | ||
| canSetShareAccess={canSetShareAccess} | ||
| hasHitSelectionLimit={hasHitSelectionLimit} | ||
| isSingleSelect={isSingleSelect} | ||
| selectableType={selectableType} | ||
| onItemSelect={onItemSelect} | ||
| onItemClick={onItemClick} | ||
| onFocusChange={onFocusChange} | ||
| onShareAccessChange={onShareAccessChange} | ||
| extensionsWhitelist={extensionsWhitelist} |
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.
can you sort these props alphabetically?
| <ProgressBar percent={currentCollection.percentLoaded} /> | ||
| )} | ||
| {isEmpty(view, currentCollection) ? ( | ||
| <EmptyView view={view} isLoading={currentCollection.percentLoaded !== 100} /> |
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.
can you sort the props alphabetically?
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.
| <EmptyView view={view} isLoading={currentCollection.percentLoaded !== 100} /> | |
| <EmptyView isLoading={currentCollection.percentLoaded !== 100} view={view} /> |
Line-by-Line Comparison between Content.tsx and Content.js.flow
The main structural changes were:
|
Line-by-Line Comparison between Content.tsx and Content.js.flow (Part 1)
|
Line-by-Line Comparison (Part 2)
|
Line-by-Line Comparison (Part 3)
|
Line-by-Line Comparison (Summary)The main structural changes were:
|
|
Closing due to inactivity. |
Description
Converted Content.js to TypeScript in the content-picker element.
Changes made:
Link to Devin run: https://app.devin.ai/sessions/848dfdad3fe447138f3a218f3058e660
Requested by: [email protected]