-
Notifications
You must be signed in to change notification settings - Fork 12
fix: default image in workspaces for image types #2108
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
Conversation
🦋 Changeset detectedLatest commit: 1e2e10b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
packages/common/src/core/schemas/internal/getThumbnailUiSchemaElement.ts
Show resolved
Hide resolved
packages/common/src/core/schemas/internal/getThumbnailUiSchemaElement.ts
Show resolved
Hide resolved
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.
Pull Request Overview
This PR fixes the default thumbnail image display for Image-type items in workspaces by passing the item's data URL as the default thumbnail. The solution leverages the existing arcgis-hub-image-picker component rather than adding new custom logic.
Key Changes:
- Added
getDefaultImageEntityThumbnailhelper function to determine if content is an Image type and return its data URL - Modified
getThumbnailUiSchemaElementto accept an optionaldefaultThumbnailUrlparameter - Updated
buildUiSchemain ContentUiSchemaEdit to pass the Image entity's data URL as the default thumbnail
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/common/src/core/schemas/internal/getThumbnailUiSchemaElement.ts | Added getDefaultImageEntityThumbnail helper and updated function signature to accept optional default thumbnail URL |
| packages/common/src/content/_internal/ContentUiSchemaEdit.ts | Updated thumbnail UI schema call to include default image for Image-type content |
| packages/common/test/core/schemas/internal/getThumbnailUiSchemaElement.spec.ts | Added tests for new getDefaultImageEntityThumbnail function |
| .changeset/giant-poets-fall.md | Added changeset documenting the fix |
packages/common/src/core/schemas/internal/getThumbnailUiSchemaElement.ts
Show resolved
Hide resolved
packages/common/src/core/schemas/internal/getThumbnailUiSchemaElement.ts
Show resolved
Hide resolved
packages/common/test/core/schemas/internal/getThumbnailUiSchemaElement.spec.ts
Show resolved
Hide resolved
knbhagat
left a comment
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.
LGTM - Great job Aaron
Description:
I realized there was a much nicer way to do this vs what I thought was needed previously. All we need to do is pass in a default thumbnail image string if the item is of type
Image. Thearcgis-hub-image-pickerdoes all the work that I thought we'd need to add around the default imageInstructions for testing: see https://github.com/ArcGIS/opendata-ui/pull/15367
Closes Issues: https://devtopia.esri.com/dc/hub/issues/14191
Updated meaningful TSDoc to methods including Parameters and Returns, see Documentation Guide
Either ran
npm run changesetor this change does not require a release.These changes have been verified by QA using a
?uiVersionthat includes a PR-Preview of this branch and thev_reqlabel has been applied to the issue.OD-UI E2E tests pass against these changes using a
?uiVersionthat includes a PR-Preview of this branch