Skip to content

Conversation

@devin-ai-integration
Copy link
Contributor

Description

This PR adds an onSelection callback to the ContentUploader component to allow developers to control file selection before upload begins. This implements the suggestion from issue #3839.

Changes

  • Added onSelection prop to ContentUploader component
  • Added TypeScript and Flow type definitions
  • Implemented file selection validation in UploadInput component
  • Added comprehensive test coverage for the new callback

Usage

<ContentUploader
  onSelection={(files) => {
    // Return false to prevent upload
    // Return true to allow upload to proceed
    return files.length <= 5; // Example: Only allow up to 5 files
  }}
  {...otherProps}
/>

Testing

  • Added unit tests for UploadInput component
  • Verified all tests pass
  • Ran lint checks via precommit hooks

Link to Devin run: https://app.devin.ai/sessions/acbecd8da4154ad8aba8d8d81d7bd661

@devin-ai-integration devin-ai-integration bot requested review from a team as code owners January 13, 2025 22:22
@devin-ai-integration
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add "(aside)" to your comment to have me ignore it.
  • Look at CI failures and help fix them

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

directory={isFolderUpload ? '' : undefined}
multiple={isMultiple}
onChange={onChange}
onChange={(event: React.ChangeEvent<HTMLInputElement>) => {

Choose a reason for hiding this comment

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

That comes handy when trying to upload files and their size exceeded 50mb (5010001000 as I figured Box doesn't like 5010241024 too much ;)).
Currnetly I handled it by observing classList string and reacting to the change after the onBeforeUpload fired and contentExplorer changed it's button classes etc.

if (onSelection && files) {
const shouldContinue = onSelection(files);
if (!shouldContinue) {
return;

Choose a reason for hiding this comment

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

Is there a way to clear out the file list other than doing files = ''?
I know there's clearCache for remote but it'd be helpful to be able to reset the form other than clicking cancel.

Lukortech added a commit to Lukortech/developer.box.com that referenced this pull request Jan 14, 2025
I've found some information scattered around the web about this UI element but the documentation doesn't mention this event and even more intruiguing is that it doesn't really work as I would expect it to. 

I should be able to validate files (like exceeded size, fileType) before even allowing user to click "upload" button. That's not the case and I've already got davin to write some code: 

box/box-ui-elements#3841
be.versionDeleted = {name} v{version_number}মুছে দিয়েছে
# Message displayed in the activity feed to represent the range of versions actioned by multiple users. {numberOfCollaborators} is a number. {actionMessage} is the action. {versions} is a range of versions.
be.versionMultipleUsersCollapsed = {numberOfCollaborators} collaborators {actionMessage} v{versions}
be.versionMultipleUsersCollapsed = {numberOfCollaborators} কোলাবোরেটর {actionMessage} v{versions}
Copy link
Contributor

Choose a reason for hiding this comment

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

is it trying to handle it's own translations?

Choose a reason for hiding this comment

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

Certainly looks like it.

Choose a reason for hiding this comment

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

Do we need to address that or is the PR good to go?

@devin-ai-integration
Copy link
Contributor Author

Closing due to inactivity.

@Lukortech
Copy link

Closing due to inactivity.

I don't think so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants