-
Notifications
You must be signed in to change notification settings - Fork 0
feat(arf): add resizable image inputs #225
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: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,6 +12,7 @@ import { Button } from "@/components/ui/button"; | |||||||||||||
| import { FormControl, FormLabel } from "@/components/ui/form"; | ||||||||||||||
| import { Input } from "@/components/ui/input"; | ||||||||||||||
| import { ImageType } from "@/config/enums"; | ||||||||||||||
| import type { ImageSize } from "@/features/abstract-resource-form"; | ||||||||||||||
| import { ApiImage, uploadFile, useMutationWrapper } from "@/features/backend"; | ||||||||||||||
| import { declineNoun } from "@/features/polish"; | ||||||||||||||
| import type { Resource } from "@/features/resources"; | ||||||||||||||
|
|
@@ -20,15 +21,40 @@ import type { | |||||||||||||
| ResourceSchemaKey, | ||||||||||||||
| } from "@/features/resources/types"; | ||||||||||||||
| import { getToastMessages } from "@/lib/get-toast-messages"; | ||||||||||||||
| import { cn } from "@/lib/utils"; | ||||||||||||||
| import type { WrapperProps } from "@/types/components"; | ||||||||||||||
|
|
||||||||||||||
| import { ImagePreviewModal } from "../presentation/image-preview-modal"; | ||||||||||||||
|
|
||||||||||||||
| function InputBox({ children }: WrapperProps) { | ||||||||||||||
| function getImageSizeClasses(size: ImageSize): string { | ||||||||||||||
| switch (size) { | ||||||||||||||
| case "small": { | ||||||||||||||
| return "md:size-32"; | ||||||||||||||
| } | ||||||||||||||
| case "medium": { | ||||||||||||||
| return "md:size-48"; | ||||||||||||||
| } | ||||||||||||||
| case "large": { | ||||||||||||||
| return "md:size-64"; | ||||||||||||||
| } | ||||||||||||||
| case "wide": { | ||||||||||||||
| return "md:h-48 md:w-full aspect-video"; | ||||||||||||||
| } | ||||||||||||||
|
||||||||||||||
| } | |
| } | |
| default: { | |
| const _exhaustiveCheck: never = size; | |
| throw new Error(`Unhandled image size: ${_exhaustiveCheck}`); | |
| } |
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.
to nie powinna być funkcja ze switchem tylko obiekt jako mapa
Copilot
AI
Jan 8, 2026
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.
There's a potential CSS conflict for small/medium/large sizes. The aspect-video class (line 55) applies at all breakpoints, while size-specific classes like md:size-32 (line 56) set explicit square dimensions at md+ breakpoint. This creates competing constraints where aspect-video requests a 16:9 ratio but md:size-32 sets equal width and height. Consider applying aspect-video only below the md breakpoint by using max-md:aspect-video, or clarify if the current behavior where explicit dimensions override aspect ratio is intentional.
| size !== "wide" && "aspect-video max-h-48", | |
| size !== "wide" && "max-md:aspect-video max-h-48", |
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.
użyj notacji { "aspect-vide max-h-48": size !== "wide" } zamiast && dla spójności z resztą kodu
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.
to zły pomysł żeby to były string literały. zmień to na enuma
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -5,3 +5,5 @@ export * from "./components/abstract-resource-form"; | |||
| export * from "./hooks/use-arf-sheet"; | ||||
|
|
||||
| export * from "./providers/arf-sheet-provider"; | ||||
|
|
||||
| export type { ImageSize } from "./types/inputs"; | ||||
|
||||
| export type { ImageSize } from "./types/inputs"; |
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.
The "wide" size applies
md:h-48,md:w-full, andaspect-videosimultaneously. This creates a conflict:md:w-fullmakes the element full width,aspect-videowants to maintain a 16:9 ratio, butmd:h-48fixes the height at 48px. These constraints cannot all be satisfied simultaneously. Consider either removingmd:h-48to let the aspect ratio control the height based on the full width, or replacingaspect-videowith a max-height constraint if a fixed height is desired.