feat: add ProgressGranularity #1151
Conversation
🦋 Changeset detectedLatest commit: ff2120b The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThis pull request refines the upload progress handling across multiple components. Updates include modifications to the styling and progress display in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
Scope: all 3 workspace projects This error happened while installing a direct dependency of /tmp/eslint/packages/uploadthing Packages found in the workspace: 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (6)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
📦 Bundle size comparison
|
More templates
@uploadthing/nuxt
@uploadthing/react
@uploadthing/solid
@uploadthing/shared
@uploadthing/svelte
uploadthing
@uploadthing/vue
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/shared/src/component-utils.ts (1)
18-26: LGTM! Consider adding input validation.The implementation is clean and effective. Consider adding input validation to ensure the progress value is within the valid range [0-100].
export const roundProgress = ( progress: number, granularity: ProgressGranularity, ) => { + const clampedProgress = Math.max(0, Math.min(100, progress)); if (granularity === "all") return progress; - if (granularity === "fine") return Math.round(progress); - return Math.floor(progress / 10) * 10; + if (granularity === "fine") return Math.round(clampedProgress); + return Math.floor(clampedProgress / 10) * 10; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/react/src/components/button.tsx(6 hunks)packages/react/src/components/dropzone.tsx(6 hunks)packages/react/src/components/shared.tsx(0 hunks)packages/react/src/types.ts(2 hunks)packages/react/src/use-uploadthing.ts(3 hunks)packages/shared/src/component-utils.ts(1 hunks)packages/uploadthing/src/_internal/upload-browser.ts(1 hunks)playground/components/uploader.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- packages/react/src/components/shared.tsx
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: typecheck
- GitHub Check: lint
- GitHub Check: e2e-node (backend-adapters)
- GitHub Check: e2e-node (minimal-pagedir)
- GitHub Check: build
- GitHub Check: e2e-node (minimal-appdir)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (12)
playground/components/uploader.tsx (1)
81-81: LGTM!The addition of
uploadProgressGranularity="fine"will provide more granular progress updates in 1% increments, enhancing the user experience.packages/react/src/types.ts (1)
7-7: LGTM!The new
uploadProgressGranularityproperty is well-documented with clear JSDoc comments explaining the available options and their effects. The type is properly imported and integrated into the existing type system.Also applies to: 67-74
packages/react/src/use-uploadthing.ts (1)
11-11: LGTM! Progress calculation improvements.The changes enhance the upload progress calculation by:
- Preventing the total progress from exceeding 100%
- Using the new
roundProgressfunction for consistent rounding based on the selected granularity- Maintaining backward compatibility with a default "coarse" granularity
Also applies to: 66-66, 99-104
packages/uploadthing/src/_internal/upload-browser.ts (1)
211-211: LGTM! Progress calculation now supports granular updates.The removal of
Math.roundfrom progress calculations enables more granular progress updates, allowing for smoother progress visualization and better control over progress display through theuploadProgressGranularityproperty.Also applies to: 215-215
packages/react/src/components/button.tsx (4)
3-3: LGTM! Added CSSProperties type for better type safety.The import of
CSSPropertiesfrom React ensures proper type checking for the style object.
127-127: LGTM! Added uploadProgressGranularity for finer control.The
uploadProgressGranularityproperty is now passed to the internal upload function, enabling more granular control over progress updates.
252-254: LGTM! Improved progress display with rounding.The progress value is now rounded before display, ensuring a cleaner presentation of the percentage.
319-324: LGTM! Enhanced progress visualization using CSS variable.The progress width is now controlled through a CSS variable
--progress-width, providing a more maintainable and performant way to handle progress visualization.Also applies to: 333-333
packages/react/src/components/dropzone.tsx (4)
13-13: LGTM! Added CSSProperties type for better type safety.The import of
CSSPropertiesfrom React ensures proper type checking for the style object.
159-159: LGTM! Added uploadProgressGranularity for finer control.The
uploadProgressGranularityproperty is now passed to the internal upload function, enabling more granular control over progress updates.
284-286: LGTM! Improved progress display with rounding.The progress value is now rounded before display, ensuring a cleaner presentation of the percentage.
311-316: LGTM! Enhanced progress visualization using CSS variable.The progress width is now controlled through a CSS variable
--progress-width, providing a more maintainable and performant way to handle progress visualization.Also applies to: 383-383
vercel
bot
temporarily deployed
to
Preview – legacy-docs-uploadthing
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/svelte/src/lib/types.ts (1)
74-74: Consider adding examples to the documentation.While the documentation is clear, adding usage examples would further improve developer experience.
Add examples to the JSDoc:
* - "coarse" - Progress is reported in increments of 10% * @default "coarse" + * @example + * ```ts + * uploadProgressGranularity: "fine" // Reports progress in 1% increments + * ``` */packages/svelte/src/lib/component/UploadDropzone.svelte (1)
266-268: LGTM! Clean implementation of progress indicator using CSS variables.The changes improve the implementation by:
- Using CSS variables for progress width instead of static classes
- Adding proper state management with data attributes
- Rounding progress numbers for cleaner display
Consider using a CSS custom property name that better reflects its purpose:
- style={`--progress-width: ${uploadProgress}%; ${styleFieldToClassName(appearance?.button, styleFieldArg)}`} + style={`--upload-progress-width: ${uploadProgress}%; ${styleFieldToClassName(appearance?.button, styleFieldArg)}`}And update the corresponding reference:
- "after:absolute after:left-0 after:h-full after:w-[var(--progress-width)] after:bg-blue-600 after:transition-[width] after:duration-500 after:content-['']", + "after:absolute after:left-0 after:h-full after:w-[var(--upload-progress-width)] after:bg-blue-600 after:transition-[width] after:duration-500 after:content-['']",Also applies to: 270-271, 274-274, 289-291
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
packages/solid/src/components/button.tsx(4 hunks)packages/solid/src/components/dropzone.tsx(4 hunks)packages/solid/src/components/shared.tsx(0 hunks)packages/solid/src/create-uploadthing.ts(3 hunks)packages/solid/src/types.ts(2 hunks)packages/svelte/src/lib/component/UploadButton.svelte(7 hunks)packages/svelte/src/lib/component/UploadDropzone.svelte(5 hunks)packages/svelte/src/lib/component/shared.ts(0 hunks)packages/svelte/src/lib/create-uploadthing.ts(3 hunks)packages/svelte/src/lib/types.ts(2 hunks)packages/vue/src/components/button.tsx(6 hunks)packages/vue/src/components/dropzone.tsx(5 hunks)packages/vue/src/components/shared.tsx(0 hunks)packages/vue/src/types.ts(2 hunks)packages/vue/src/useUploadThing.ts(3 hunks)
💤 Files with no reviewable changes (3)
- packages/solid/src/components/shared.tsx
- packages/svelte/src/lib/component/shared.ts
- packages/vue/src/components/shared.tsx
🧰 Additional context used
🧠 Learnings (1)
packages/solid/src/components/dropzone.tsx (1)
Learnt from: juliusmarminge
PR: pingdotgg/uploadthing#977
File: packages/solid/src/components/dropzone.tsx:539-544
Timestamp: 2024-11-12T10:36:58.532Z
Learning: In `packages/solid/src/components/dropzone.tsx`, issues related to the `onClick` handler definition will be fixed in PR #980.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: e2e-node (backend-adapters)
🔇 Additional comments (31)
packages/solid/src/types.ts (2)
7-7: LGTM!The
ProgressGranularitytype import is correctly placed and follows the existing import pattern.
67-74: LGTM!The
uploadProgressGranularityproperty is well-documented with clear explanations for each value and its default. The type definition follows best practices by being optional and including undefined in the union type.packages/svelte/src/lib/create-uploadthing.ts (3)
7-7: LGTM!The
roundProgressimport is correctly added to support the new progress granularity feature.
44-44: LGTM!The
progressGranularityinitialization with a default value of "coarse" aligns with the type definition and provides a sensible default.
77-82: LGTM!The progress calculation improvements are well-implemented:
- Using
Math.minprevents the progress from exceeding 100%- The
roundProgressfunction ensures consistent granularity based on user preferencepackages/svelte/src/lib/types.ts (2)
7-7: LGTM!The
ProgressGranularitytype is correctly imported from the shared package.
67-74: LGTM!The JSDoc comments for
uploadProgressGranularityare comprehensive and well-written:
- Clear explanation of the feature's purpose
- Detailed description of each granularity option
- Default value is explicitly documented
packages/vue/src/useUploadThing.ts (3)
13-13: LGTM!The
roundProgressfunction is imported from the shared package to ensure consistent progress rounding behavior across the application.
67-67: LGTM!The
progressGranularityvariable is correctly initialized with a default value of "coarse" from the options.
100-105: LGTM!The progress calculation has been improved with:
Math.min(100, sum + p)to prevent exceeding 100%.roundProgressfunction to ensure consistent rounding based on the granularity setting.packages/vue/src/types.ts (1)
7-7: LGTM!The
uploadProgressGranularityproperty is well-documented with clear JSDoc comments explaining the available options and default value.Also applies to: 74-74
packages/svelte/src/lib/component/UploadButton.svelte (4)
16-16: LGTM!The imports are correctly added to support type safety and CSS object generation.
Also applies to: 22-22
81-81: LGTM!The
uploadProgressGranularityproperty is correctly passed to the internal upload function.
159-159: LGTM!The style binding now uses a CSS variable for progress width, improving maintainability and performance.
223-225: LGTM!The progress display is improved with:
- Proper rounding using
Math.round.- Clear block/hidden states for hover interaction.
packages/solid/src/components/button.tsx (3)
107-107: LGTM!The
uploadProgressGranularityproperty is correctly passed to the internal upload function.
224-227: LGTM!The style binding now uses a CSS variable for progress width and correctly spreads the CSS object from
styleFieldToCssObject.
209-211: LGTM!The progress display is improved with:
- Proper rounding using
Math.round.- Clear block/hidden states for hover interaction.
packages/vue/src/components/button.tsx (3)
99-100: LGTM! Progress granularity support added.The
uploadProgressGranularityproperty is correctly passed to the internal upload handler.Also applies to: 123-124
219-220: LGTM! Progress display now uses rounded values.The upload progress percentage is now rounded for better user experience.
310-316: LGTM! Progress styling uses CSS variable.The progress width is now controlled via a CSS variable, providing better styling flexibility.
packages/solid/src/components/dropzone.tsx (3)
132-133: LGTM! Progress granularity support added.The
uploadProgressGranularityproperty is correctly passed to the internal upload handler.
342-343: LGTM! Progress display now uses rounded values.The upload progress percentage is now rounded for better user experience.
307-314: LGTM! Progress styling uses CSS variable.The progress width is now controlled via a CSS variable, providing better styling flexibility.
packages/vue/src/components/dropzone.tsx (3)
123-124: LGTM! Progress granularity support added.The
uploadProgressGranularityproperty is correctly passed to the internal upload handler.
315-316: LGTM! Progress display now uses rounded values.The upload progress percentage is now rounded for better user experience.
385-391: LGTM! Progress styling uses CSS variable.The progress width is now controlled via a CSS variable, providing better styling flexibility.
packages/solid/src/create-uploadthing.ts (2)
11-11: LGTM!The
roundProgressimport is correctly added and aligns with the new progress calculation logic.
53-53: LGTM!The
progressGranularityvariable is well-placed and properly initialized with a sensible default value.packages/svelte/src/lib/component/UploadDropzone.svelte (2)
33-33: LGTM!The import of
getFilesFromClipboardEventis well-named and appropriately placed.
161-161: LGTM! Good use of AbortController for event cleanup.The implementation ensures proper cleanup of the paste event listener when the component is destroyed.
Also applies to: 177-177
|
A new canary is available for testing. You can install this latest build in your project with: pnpm add @uploadthing/[email protected]
pnpm add @uploadthing/[email protected]
pnpm add @uploadthing/[email protected]
pnpm add @uploadthing/[email protected]
pnpm add @uploadthing/[email protected]
pnpm add @uploadthing/[email protected]
pnpm add @uploadthing/[email protected]
pnpm add [email protected]
pnpm add @uploadthing/[email protected] |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.changeset/fair-tomatoes-jog.md (2)
10-12: Commit Message Clarity NoteThe commit message "feat: add
progressGranularityoption to control how granular progress events are fired at" is descriptive. However, consider rephrasing "fired at" to simply "fired" or "triggered" to enhance clarity.
16-20: Clarify the Granularity BehaviorsThe bullet-point style description effectively outlines how each granularity level behaves:
all: forwards every XHR event.find: triggers events for every 1% progress.coarse: triggers events for every 10% progress.Consider adding a note on whether the values are case sensitive to prevent any potential user confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/fair-tomatoes-jog.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build & Publish a canary release
- GitHub Check: e2e-node (backend-adapters)
- GitHub Check: build
🔇 Additional comments (2)
.changeset/fair-tomatoes-jog.md (2)
1-8: Version Bump Declarations Are ClearThe package version bump entries are clearly declared with appropriate changetype markers (
minor&patch). Please verify that these version updates align with the overall semantic versioning strategy for the project.
13-15: Detailed Explanation for Progress GranularityThe description clearly explains the new
progressGranularityoption and lists the allowed values (all,find,coarse). The message is concise and informative.
| `bg-blue-400 after:absolute after:left-0 after:h-full after:bg-blue-600 after:content-[''] ${progressWidths[uploadProgress]}`, | ||
| state === "ready" && "bg-blue-600", | ||
| "disabled:pointer-events-none", | ||
| "data-[state=disabled]:cursor-not-allowed data-[state=readying]:cursor-not-allowed", |
Co-authored-by: Mark R. Florkowski <[email protected]>
Summary by CodeRabbit
New Features
uploadProgressGranularityto the upload button, dropzone, and related components for improved tracking options.Refactor
Bug Fixes