[WEB-5423] fix: typescript errors and add types check step to pull request workflow #8110
Conversation
WalkthroughAdds a CI type-check step, updates ESLint/Prettier ignore lists, deletes installation OAuth layout/page, migrates some router/type imports, relaxes projectId parameter types, removes explicit navigation progress flags, enhances drag-and-drop behavior and imports, and updates Storybook stories and a spinner interface export. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Draggable as Draggable Component
participant Monitor as ClosestEdge Monitor
participant Indicator as DropIndicator
User->>Draggable: dragStart()
Draggable->>Draggable: set dragging = true
User->>Draggable: dragEnter(data)
Draggable->>Monitor: attachClosestEdge(data, allowedEdges:[top,bottom])
Monitor-->>Draggable: closestEdge
Draggable->>Draggable: set isDraggedOver = true, record closestEdge
Draggable->>Indicator: show at closestEdge
User->>Draggable: dragLeave()
Draggable->>Draggable: set isDraggedOver = false
Draggable->>Indicator: hide
User->>Draggable: drop(payload)
alt payload.__uuid__ matches and source ≠ target
Draggable->>Draggable: accept drop -> process reorder
else
Draggable->>Draggable: reject drop
end
Draggable->>Draggable: set dragging = false, isDraggedOver = false
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
prateekshourya29
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/propel/src/emoji-icon-picker/emoji-picker.stories.tsx (1)
20-384: Remove unusedargsfrom all stories.Every story defines an
argsobject withisOpen,handleToggle,onChange, andlabel, but none of these are referenced in therender()functions. Each render function creates its own local state viauseStateand passes those local values to theEmojiPickercomponent instead.In Storybook, args are meant to control story behavior, but here they serve no purpose and are dead code that could mislead developers.
Apply this diff to remove the unused args from all stories:
export const Default: Story = { - args: { - isOpen: false, - handleToggle: () => {}, - onChange: () => {}, - label: "Default", - }, render() {Repeat this pattern for all 11 stories:
Default,OpenToEmojiTab,OpenToIconTab,LucideIcons,MaterialIcons,CloseOnSelectDisabled,CustomSearchPlaceholder,SearchDisabled,CustomIconColor,DifferentPlacements, andInFormContext.Note: If the intent was to expose interactive controls in Storybook, you would need to refactor the render functions to use the args instead of local state. However, for components like this that require complex stateful interactions, the current approach of using local state in render functions is appropriate—just without the unused args declarations.
packages/ui/src/sortable/draggable.tsx (2)
20-51: Critical: Missing cleanup function causes memory leaks.The
combinefunction returns a cleanup function that must be returned from theuseEffectto properly unregister drag-and-drop event listeners. Without this cleanup, event listeners will accumulate on every re-render whendatachanges, leading to memory leaks and potential duplicate event handling.Apply this diff to add the cleanup function:
useEffect(() => { const el = ref.current; if (el) { - combine( + return combine( draggable({ element: el, onDragStart: () => setDragging(true), // NEW onDrop: () => setDragging(false), // NEW getInitialData: () => data, }), dropTargetForElements({ element: el, onDragEnter: (args) => { setIsDraggedOver(true); setClosestEdge(extractClosestEdge(args.self.data)); }, onDragLeave: () => setIsDraggedOver(false), onDrop: () => { setIsDraggedOver(false); }, canDrop: ({ source }) => !isEqual(source.data, data) && source.data.__uuid__ === data.__uuid__, getData: ({ input, element }) => attachClosestEdge(data, { input, element, allowedEdges: ["top", "bottom"], }), }) ); } }, [data]);
41-41: Add null safety to__uuid__access.The
canDroplogic accessessource.data.__uuid__anddata.__uuid__without checking if these properties exist. If__uuid__is missing, this could lead to unexpected behavior or runtime errors.Apply this diff to add safety checks:
- canDrop: ({ source }) => !isEqual(source.data, data) && source.data.__uuid__ === data.__uuid__, + canDrop: ({ source }) => + !isEqual(source.data, data) && + source.data?.__uuid__ != null && + data?.__uuid__ != null && + source.data.__uuid__ === data.__uuid__,Alternatively, if
__uuid__is always expected to be present, add a runtime assertion or TypeScript type guard.
🧹 Nitpick comments (3)
packages/propel/src/calendar/calendar.stories.tsx (1)
186-194: Maintain consistency with other stories by accepting theargsparameter.The
Uncontrolledstory is the only one in this file that doesn't accept theargsparameter or spread it into the component. This creates an inconsistency with the other 8 stories and removes the ability for users to interact with this story via Storybook controls (e.g., togglingshowOutsideDays).Apply this diff to maintain consistency:
export const Uncontrolled: Story = { - render() { + render(args) { return ( <div className="p-4"> - <Calendar mode="single" defaultMonth={new Date(2024, 0)} showOutsideDays className="rounded-md border" /> + <Calendar {...args} mode="single" defaultMonth={new Date(2024, 0)} className="rounded-md border" /> </div> ); }, };Note: "Uncontrolled" refers to the React state management pattern (no
selected/onSelectprops), not the Storybook args pattern. Even uncontrolled React components should accept Storybook args for interactive controls.packages/ui/src/sortable/draggable.tsx (2)
11-11: Address theanytype for better type safety.Since this PR specifically targets TypeScript errors and type checking, consider replacing
anywith a generic type parameter to maintain type safety throughout the drag-and-drop operations.Apply this diff to introduce a generic type:
-type Props = { +type Props<T = Record<string, unknown>> = { children: React.ReactNode; - data: any; //@todo make this generic + data: T & { __uuid__?: string }; className?: string; }; -const Draggable = ({ children, data, className }: Props) => { +const Draggable = <T = Record<string, unknown>>({ children, data, className }: Props<T>) => {
19-19: Tighten the type forclosestEdge.The
closestEdgestate is typed asstring | null, but based on theallowedEdges: ["top", "bottom"]configuration on line 46, it should have a more specific type.Apply this diff to improve type safety:
- const [closestEdge, setClosestEdge] = useState<string | null>(null); + const [closestEdge, setClosestEdge] = useState<"top" | "bottom" | null>(null);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.github/workflows/pull-request-build-lint-web-apps.yml(1 hunks)apps/admin/.eslintignore(1 hunks)apps/space/.eslintignore(1 hunks)apps/web/app/(all)/installations/[provider]/layout.tsx(0 hunks)apps/web/app/(all)/installations/[provider]/page.tsx(0 hunks)apps/web/ce/components/command-palette/power-k/context-detector.ts(1 hunks)apps/web/ce/store/user/permission.store.ts(1 hunks)apps/web/core/components/cycles/cycle-peek-overview.tsx(1 hunks)apps/web/core/components/cycles/list/cycle-list-item-action.tsx(1 hunks)apps/web/core/components/cycles/list/cycles-list-item.tsx(1 hunks)apps/web/core/components/power-k/core/context-detector.ts(1 hunks)apps/web/core/components/power-k/core/types.ts(2 hunks)apps/web/core/components/project/card.tsx(1 hunks)apps/web/core/store/user/base-permissions.store.ts(1 hunks)packages/propel/src/calendar/calendar.stories.tsx(1 hunks)packages/propel/src/emoji-icon-picker/emoji-picker.stories.tsx(11 hunks)packages/propel/src/spinners/circular-bar-spinner.tsx(1 hunks)packages/ui/src/sortable/draggable.tsx(1 hunks)packages/ui/src/sortable/sortable.tsx(1 hunks)
💤 Files with no reviewable changes (2)
- apps/web/app/(all)/installations/[provider]/layout.tsx
- apps/web/app/(all)/installations/[provider]/page.tsx
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-10-06T01:44:38.472Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/collapsible/collapsible.stories.tsx:4-4
Timestamp: 2025-10-06T01:44:38.472Z
Learning: In Storybook v9, imports use bare paths instead of scoped packages. For example, `import { useArgs } from "storybook/preview-api"` is correct (not `storybook/preview-api`). This applies to other Storybook modules as well - the scoped storybook/* packages were consolidated into bare "storybook/*" imports.
Applied to files:
packages/ui/src/sortable/sortable.tsxpackages/propel/src/calendar/calendar.stories.tsxpackages/propel/src/emoji-icon-picker/emoji-picker.stories.tsx
📚 Learning: 2025-06-16T07:23:39.497Z
Learnt from: vamsikrishnamathala
Repo: makeplane/plane PR: 7214
File: web/core/store/issue/helpers/base-issues.store.ts:117-117
Timestamp: 2025-06-16T07:23:39.497Z
Learning: In the updateIssueDates method of BaseIssuesStore (web/core/store/issue/helpers/base-issues.store.ts), the projectId parameter is intentionally made optional to support override implementations in subclasses. The base implementation requires projectId and includes an early return check, but making it optional allows derived classes to override the method with different parameter requirements.
Applied to files:
apps/web/core/store/user/base-permissions.store.tsapps/web/ce/store/user/permission.store.ts
📚 Learning: 2025-07-08T13:41:01.659Z
Learnt from: prateekshourya29
Repo: makeplane/plane PR: 7363
File: apps/web/core/components/issues/select/dropdown.tsx:9-11
Timestamp: 2025-07-08T13:41:01.659Z
Learning: The `getProjectLabelIds` function in the label store handles undefined projectId internally, so it's safe to pass undefined values to it without explicit checks in the calling component.
Applied to files:
apps/web/core/store/user/base-permissions.store.tsapps/web/ce/store/user/permission.store.ts
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
apps/web/ce/components/command-palette/power-k/context-detector.tsapps/web/core/components/power-k/core/types.tsapps/web/core/components/power-k/core/context-detector.ts
📚 Learning: 2025-10-09T20:42:31.843Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Applied to files:
apps/web/core/components/power-k/core/context-detector.ts
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/core/components/power-k/core/context-detector.ts
🧬 Code graph analysis (3)
apps/web/core/components/project/card.tsx (1)
apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
apps/web/core/store/user/base-permissions.store.ts (1)
apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
packages/propel/src/calendar/calendar.stories.tsx (1)
packages/propel/src/calendar/root.tsx (1)
Calendar(11-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and lint web apps
🔇 Additional comments (15)
apps/space/.eslintignore (1)
2-3: Appropriate additions for build artifact exclusion.Adding
.react-router/*and.vite/*to the eslint ignore list makes sense given the migration to React Router and use of Vite as a build tool. These directories contain generated and cached artifacts that should not be subject to linting.apps/admin/.eslintignore (1)
2-3: Consistent with apps/space/.eslintignore.The same build artifact exclusions are appropriately applied here for consistency across the monorepo workspaces. Both
.react-router/*and.vite/*should be safely ignored from linting.packages/propel/src/spinners/circular-bar-spinner.tsx (1)
3-7: LGTM! Appropriate type export for the component library.Exporting the
ICircularBarSpinnerinterface allows consumers of this component to properly type-check props when using the component. This is a standard practice for shared component libraries and aligns with the PR's objective to fix TypeScript errors..github/workflows/pull-request-build-lint-web-apps.yml (1)
52-53: New type-check step is well-placed in the workflow.The new step follows the existing patterns, uses the appropriate turbo command syntax, and is logically positioned—after format checks but before the build—to catch type errors early in the CI pipeline. The
check:typestask is properly configured inturbo.jsonwith appropriate build dependencies and caching disabled.packages/ui/src/sortable/sortable.tsx (1)
1-1: LGTM! Clean import path modernization.The import path update from the internal CJS dist path to the public module export is a good improvement. This makes the code more maintainable and resilient to library internals changes.
apps/web/core/store/user/base-permissions.store.ts (1)
115-122: LGTM! TypeScript type alignment fix.Making
projectIdoptional aligns the implementation with the interface declaration (lines 35-38, 146-149) that already specifiesprojectId?: string. The guard at line 116 safely handles undefined by returningundefined, and all call sites either pass defined values or guard appropriately before calling.Based on learnings.
apps/web/ce/store/user/permission.store.ts (1)
20-23: LGTM! Consistent with base class change.The optional
projectIdparameter correctly aligns with the base classgetProjectRolemethod signature change and matches the abstract method contract. The forwarding tothis.getProjectRolesafely delegates undefined handling to the base implementation.apps/web/core/components/cycles/list/cycle-list-item-action.tsx (1)
199-201: Navigation options removed—verify progress indicator behavior.The removal of navigation options (previously
showProgress: false) is consistent with the router API migration. Confirm that the default progress indicator behavior is acceptable for the cycle overview navigation flow.apps/web/core/components/project/card.tsx (1)
122-122: Navigation options removed—verify UX consistency.The settings navigation now uses default progress behavior. Ensure this aligns with user expectations for context menu actions across the application.
apps/web/core/components/cycles/cycle-peek-overview.tsx (1)
34-34: Navigation options removed—verify close transition.The close action now uses default progress behavior. Confirm that this provides a smooth user experience when dismissing the cycle peek overlay.
apps/web/core/components/cycles/list/cycles-list-item.tsx (1)
62-64: Navigation options removed—verify progress indicator behavior.The removal of navigation options is consistent with the router API migration. Confirm that the default progress indicator behavior meets UX requirements for cycle list item navigation.
packages/ui/src/sortable/draggable.tsx (2)
1-3: LGTM!The imports are correctly updated to use the new module paths for the pragmatic-drag-and-drop library and its hitbox utilities.
53-59: LGTM!The render logic correctly applies visual feedback during drag operations with opacity changes and position-specific drop indicators based on the closest edge detection.
apps/web/core/components/power-k/core/types.ts (1)
1-1: Router type migration is safe and properly implemented.Verification confirms:
- All usages of
context.routeronly call.push()(standard Next.js router API)- No
@bprogress/nextdependency exists in the codebase- No progress-specific router methods are invoked anywhere
TPowerKContext.routeris correctly typed asReturnType<typeof useRouter>fromnext/navigationThe migration from progress-wrapped to native Next.js router is complete with full consumer compatibility and no breaking changes.
apps/web/core/components/power-k/core/context-detector.ts (1)
1-1: Verify manual inspection of this component's pattern against learning context.The
react-routerParamstype structure is compatible with the property names (workItem,cycleId,moduleId,pageId) as it's a generic type accepting any string keys mapping tostring | undefinedvalues. However, per learnings from this repository, the refactor fromuseParams()to aParamstype import should be scoped topage.tsxandlayout.tsxfiles in the Next.js App Router pattern. This component resides inapps/web/core/components/, which per those learnings should continue using theuseParams()hook.Verify that this component's context and usage pattern matches the intended refactor scope, or confirm whether this change represents a different, intentional pattern for CE components.
…ctories in admin and space apps
Merge branch 'fix-broken-attachment-icon-image' of github.com:makeplane/plane into fix-type-error
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/web/core/components/icons/attachment/doc-file-icon.tsx (1)
8-8: Consistent with icon refactor pattern.Same change as video-file-icon.tsx and other attachment icons. Ensure parent components handle sizing appropriately without the
object-containconstraint.
🧹 Nitpick comments (8)
apps/web/core/components/icons/attachment/txt-file-icon.tsx (1)
8-8: Verify visual rendering after removing sizing classes.Removing
h-full w-full object-containalters rendering behavior. Confirm that parent components handle icon sizing appropriately and test for visual regressions.apps/web/core/components/icons/attachment/jpg-file-icon.tsx (1)
8-8: Verify visual rendering after removing sizing classes.Confirm parent components provide appropriate sizing and test for visual regressions after removing the CSS utility classes.
apps/web/core/components/icons/attachment/zip-file-icon.tsx (1)
8-8: Verify visual rendering after removing sizing classes.Ensure this change doesn't introduce visual regressions and that parent components handle icon sizing appropriately.
apps/web/core/components/icons/attachment/pdf-file-icon.tsx (1)
8-8: Verify visual rendering after removing sizing classes.Test for visual regressions and confirm parent components provide appropriate sizing context.
apps/web/core/components/icons/attachment/sheet-file-icon.tsx (1)
8-8: Verify visual rendering after removing sizing classes.Confirm appropriate icon sizing in parent components and test for visual regressions.
apps/web/core/components/icons/attachment/css-file-icon.tsx (1)
8-8: Verify visual rendering after removing sizing classes.Ensure parent components handle icon sizing correctly and test for visual regressions after this change.
apps/web/core/components/icons/attachment/svg-file-icon.tsx (1)
8-8: Verify visual rendering after removing sizing classes.Test for visual regressions and ensure parent components provide appropriate sizing after removing CSS utility classes.
apps/web/core/components/icons/attachment/audio-file-icon.tsx (1)
5-8: Use shared ImageIconPros type for consistency.The locally defined
AudioIconPropsduplicates the sharedImageIconProstype used by other attachment icons. This creates unnecessary type duplication.Apply this diff to use the shared type:
+// type +import type { ImageIconPros } from "../types"; + -export type AudioIconProps = { - width?: number; - height?: number; -}; - -export const AudioIcon: React.FC<AudioIconProps> = ({ width, height }) => ( +export const AudioIcon: React.FC<ImageIconPros> = ({ width, height }) => (
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
apps/web/core/components/icons/attachment/audio-file-icon.tsx(1 hunks)apps/web/core/components/icons/attachment/css-file-icon.tsx(1 hunks)apps/web/core/components/icons/attachment/csv-file-icon.tsx(1 hunks)apps/web/core/components/icons/attachment/default-file-icon.tsx(1 hunks)apps/web/core/components/icons/attachment/doc-file-icon.tsx(1 hunks)apps/web/core/components/icons/attachment/figma-file-icon.tsx(1 hunks)apps/web/core/components/icons/attachment/html-file-icon.tsx(1 hunks)apps/web/core/components/icons/attachment/img-file-icon.tsx(1 hunks)apps/web/core/components/icons/attachment/jpg-file-icon.tsx(1 hunks)apps/web/core/components/icons/attachment/js-file-icon.tsx(1 hunks)apps/web/core/components/icons/attachment/pdf-file-icon.tsx(1 hunks)apps/web/core/components/icons/attachment/png-file-icon.tsx(1 hunks)apps/web/core/components/icons/attachment/rar-file-icon.tsx(1 hunks)apps/web/core/components/icons/attachment/sheet-file-icon.tsx(1 hunks)apps/web/core/components/icons/attachment/svg-file-icon.tsx(1 hunks)apps/web/core/components/icons/attachment/txt-file-icon.tsx(1 hunks)apps/web/core/components/icons/attachment/video-file-icon.tsx(1 hunks)apps/web/core/components/icons/attachment/zip-file-icon.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-04T16:22:44.344Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7164
File: packages/ui/.storybook/main.ts:24-47
Timestamp: 2025-06-04T16:22:44.344Z
Learning: In packages/ui/.storybook/main.ts, the webpackFinal function intentionally overrides the CSS loader strategy by finding and replacing existing CSS rules. This is a temporary workaround for a known upstream issue in Storybook's CSS handling that has been communicated to the Storybook maintainers. The current implementation should not be changed until the upstream issue is resolved.
Applied to files:
apps/web/core/components/icons/attachment/css-file-icon.tsx
🧬 Code graph analysis (1)
apps/web/core/components/icons/attachment/audio-file-icon.tsx (1)
packages/propel/src/icons/attachments/audio-file-icon.tsx (1)
AudioFileIcon(6-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (10)
apps/web/core/components/icons/attachment/js-file-icon.tsx (1)
8-8: Ignore this review comment—it's based on incorrect assumptions.The
JavaScriptIconcomponent uses theImageIconProstype, which only supportswidthandheightprops. TheclassNameproperty was never available in this type, so CSS classes likeh-full w-full object-containcould never have been present. The current implementation with explicitwidthandheightHTML attributes is the standard pattern across all image-based file icons in this directory.Likely an incorrect or invalid review comment.
apps/web/core/components/icons/attachment/default-file-icon.tsx (1)
8-8: LGTM - consistent with icon refactoring pattern.The simplified img rendering matches the changes across other attachment icons.
apps/web/core/components/icons/attachment/html-file-icon.tsx (1)
8-8: LGTM - consistent with icon refactoring pattern.The simplified img rendering matches the changes across other attachment icons.
apps/web/core/components/icons/attachment/img-file-icon.tsx (1)
8-8: LGTM - consistent with icon refactoring pattern.The simplified img rendering matches the changes across other attachment icons.
apps/web/core/components/icons/attachment/rar-file-icon.tsx (1)
8-8: LGTM - consistent with icon refactoring pattern.The simplified img rendering matches the changes across other attachment icons.
apps/web/core/components/icons/attachment/figma-file-icon.tsx (1)
8-8: LGTM - consistent with icon refactoring pattern.The simplified img rendering matches the changes across other attachment icons.
apps/web/core/components/icons/attachment/png-file-icon.tsx (1)
8-8: LGTM - consistent with icon refactoring pattern.The simplified img rendering matches the changes across other attachment icons.
apps/web/core/components/icons/attachment/audio-file-icon.tsx (1)
11-11: LGTM - consistent with icon refactoring pattern.The simplified img rendering matches the changes across other attachment icons.
apps/web/core/components/icons/attachment/csv-file-icon.tsx (1)
8-8: No issues found—change is safe and consistent.Verification confirms the layout change is sound. The removed Tailwind classes (
h-full w-full object-contain) were attempting to fill a container, but all usages of CsvIcon (and all attachment icons) pass explicit numeric dimensions throughgetFileIcon(). Every call site provides eithersize={28}orsize={18}, ensuring theimgelement always has proper dimensions set. This refactoring is consistent across all attachment icon components and introduces no visual regression.apps/web/core/components/icons/attachment/video-file-icon.tsx (1)
8-8: Simplified styling aligns with broader icon refactor.The removal of CSS classes in favor of intrinsic width/height attributes is consistent with 17+ other attachment icon components updated in this PR. However, removing
object-containmeans the image will no longer maintain aspect ratio constraints if the provided dimensions don't match the source image ratio. Verify that parent components pass appropriate dimensions or that the source image already has the correct aspect ratio.
Description
Type of Change
Summary by CodeRabbit
Bug Fixes
Refactor
Chores