s3 widget image support #1491
s3 widget image support #1491
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds image preview support to the S3 widget, allowing users to view thumbnails of S3-stored images directly within table views and record detail views. The implementation adds a new type parameter to distinguish between generic file storage and image-specific handling.
- Added
type?: 'file' | 'image'parameter to S3 widget configuration to enable image-specific behavior - Implemented image preview functionality with loading states for table display and record view components
- Enhanced base components with
rowDataandprimaryKeysinputs to support S3 file URL generation
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
frontend/src/app/consts/table-display-types.ts |
Added S3DisplayComponent to UIwidgets and reorganized imports with consistent formatting |
frontend/src/app/consts/record-view-types.ts |
Added S3RecordViewComponent to UIwidgets and reorganized imports with consistent formatting |
frontend/src/app/components/ui-components/table-display-fields/s3/s3.component.ts |
New component for displaying S3 files/images in table view with preview support |
frontend/src/app/components/ui-components/table-display-fields/s3/s3.component.html |
Template for S3 table display with image preview, spinner, and copy-to-clipboard functionality |
frontend/src/app/components/ui-components/table-display-fields/s3/s3.component.css |
Styling for S3 thumbnails and loading spinner in table display |
frontend/src/app/components/ui-components/table-display-fields/base-table-display-field/base-table-display-field.component.ts |
Added rowData and primaryKeys inputs to support S3 file URL generation |
frontend/src/app/components/ui-components/record-view-fields/s3/s3.component.ts |
New component for displaying S3 files/images in record detail view with preview support |
frontend/src/app/components/ui-components/record-view-fields/s3/s3.component.html |
Template for S3 record view with image preview and loading state |
frontend/src/app/components/ui-components/record-view-fields/s3/s3.component.css |
Styling for S3 preview images in record view |
frontend/src/app/components/ui-components/record-view-fields/base-record-view-field/base-record-view-field.component.ts |
Added rowData and primaryKeys inputs to support S3 file URL generation |
frontend/src/app/components/ui-components/record-edit-fields/s3/s3.component.ts |
Updated to support type parameter with file accept filtering and image type detection |
frontend/src/app/components/ui-components/record-edit-fields/s3/s3.component.html |
Added file accept attribute based on widget type configuration |
frontend/src/app/components/dashboard/db-table-view/db-table-widgets/db-table-widgets.component.ts |
Added S3 widget documentation with type parameter and security warnings |
frontend/src/app/components/dashboard/db-table-view/db-table-view.component.html |
Passed rowData and primaryKeys to dynamic display components |
frontend/src/app/components/dashboard/db-table-view/db-table-row-view/db-table-row-view.component.html |
Passed rowData and primaryKeys to dynamic record view components |
backend/src/entities/s3-widget/application/data-structures/s3-widget-params.ds.ts |
Added type parameter to S3WidgetParams interface for file/image distinction |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @Input() structure: TableField; | ||
| @Input() widgetStructure: WidgetStructure; | ||
| @Input() rowData: Record<string, unknown>; | ||
| @Input() primaryKeys: Record<string, unknown>; |
There was a problem hiding this comment.
Inconsistent type for primaryKeys between base components. In BaseTableDisplayFieldComponent, primaryKeys is typed as an array { column_name: string }[], but here it's typed as a single object Record<string, unknown>. This inconsistency will cause type mismatches when these components are used interchangeably. The type should match the structure passed from parent components and match the S3Service.getFileUrl method signature which expects Record<string, unknown>.
| type?: 'file' | 'image'; | ||
| } | ||
|
|
||
| @Injectable() |
There was a problem hiding this comment.
The @Injectable decorator should not be used here. Components don't need to be marked as @Injectable unless they're also being provided as services. Remove this decorator as it serves no purpose for a component and could cause confusion about the component's intended usage.
| <img *ngIf="previewUrl && !isLoading" | ||
| class="s3-thumbnail" | ||
| [src]="previewUrl" | ||
| alt="S3 Image"> |
There was a problem hiding this comment.
The alt text for the image is not descriptive. Using a generic "S3 Image" doesn't provide meaningful information for screen readers. Consider including the value (S3 key/filename) in the alt text to make it more descriptive, or provide context about what the image represents.
| alt="S3 Image"> | |
| [attr.alt]="value ? 'Preview of S3 object ' + value : 'S3 object preview'"> |
| alt="S3 Image"> | ||
| <span *ngIf="!previewUrl && !isLoading && value" class="field-view-value">[S3 Image]</span> |
There was a problem hiding this comment.
The alt text for the image is not descriptive. Using a generic "S3 Image" doesn't provide meaningful information for screen readers. Consider including the value (S3 key/filename) in the alt text to make it more descriptive, or provide context about what the image represents.
| alt="S3 Image"> | |
| <span *ngIf="!previewUrl && !isLoading && value" class="field-view-value">[S3 Image]</span> | |
| [attr.alt]="value ? 'S3 object preview: ' + value : 'S3 image preview'"> | |
| <span *ngIf="!previewUrl && !isLoading && value" class="field-view-value">[S3 object: {{ value }}]</span> |
| interface S3WidgetParams { | ||
| bucket: string; | ||
| prefix?: string; | ||
| region?: string; | ||
| aws_access_key_id_secret_name: string; | ||
| aws_secret_access_key_secret_name: string; | ||
| type?: 'file' | 'image'; | ||
| } |
There was a problem hiding this comment.
The S3WidgetParams interface is duplicated across three components (table-display, record-view, and record-edit). This violates the DRY principle and makes maintenance harder. Consider extracting this interface to a shared location (e.g., a models file or a shared types file) and importing it in all three components to ensure consistency and reduce duplication.
No description provided.