-
Notifications
You must be signed in to change notification settings - Fork 83
Add featured image support to Social Web reader #2506
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: add/reader
Are you sure you want to change the base?
Conversation
Implements featured image display in the ActivityPub reader feed view: - Store first image from ActivityPub attachments as post meta - Register REST API field for featured_image on ap_post post type - Create featured image field component for DataViews - Add featured_image to feed query and TypeScript types - Position image on the right side using flex layout - Clean up meta on post update The featured image is cached locally during attachment import and stored in post meta for efficient retrieval. Users can enable/disable the image field via DataViews properties panel.
|
Can you help me understand the benefit of designating a featured image? |
|
that was a feature that was requested at the meetup! to have at least one image show up in the stream view. /cc @jeherve |
|
Have at least one picture show up when excerpt is selected? |
|
yes |
Added a phpcs:ignore comment to suppress VariableAnalysis warnings for the unused $old_url variable in the inline mappings loop, clarifying that only the new URL is needed.
Integrate featured image display into the excerpt field instead of showing it as a separate column. The image now appears below the excerpt text with responsive sizing constraints. Changes: - Move featured image rendering into excerpt field component - Add SCSS styling for excerpt images (300px max dimensions, auto width/height to maintain aspect ratio) - Remove standalone featured image field from feed stage - Remove featured image export from fields index - Image displays with 1em top margin and 4px border radius - Update built assets to reflect source changes
f23b7e7 to
eb5669c
Compare
|
Updated the featured image implementation:
|
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 aims to add featured image display capability to the ActivityPub Social Web reader feed view. The implementation stores the first image from ActivityPub attachments as post meta and exposes it through the REST API for display in a new DataViews field component.
Key Changes:
- Backend stores first attachment image URL in
_activitypub_featured_image_urlpost meta - REST API exposes featured image via new
featured_imagefield onap_postpost type - Frontend creates
featuredImageFieldcomponent with associated styles - Excerpt field modified to also display featured images inline
Critical Issues:
The featured image field component is created but not properly integrated - it's missing from the fields index export and not added to the DataViews fields array, making the feature non-functional. Additionally, there's duplicate image rendering between the new featured-image field and the excerpt field, creating ambiguity about the intended design.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/social-web/types.ts | Adds optional featured_image string property to FeedPost interface |
| src/social-web/routes/feed/stage.tsx | Removes unused mediaField property from defaultLayouts (cleanup) |
| src/social-web/hooks/use-feed.ts | Adds 'featured_image' to default fields array for REST API queries |
| src/social-web/components/fields/featured-image/style.scss | New styles for featured image field display (order: 999, right-aligned) |
| src/social-web/components/fields/featured-image/index.tsx | New DataViews field component for featured images (not integrated) |
| src/social-web/components/fields/excerpt/style.scss | New styles for inline images in excerpt field |
| src/social-web/components/fields/excerpt/index.tsx | Modified to conditionally render featured images inline with excerpt text |
| includes/collection/class-posts.php | Saves first image URL to post meta; deletes meta on update before re-import |
| includes/class-post-types.php | Registers post meta and REST field for _activitypub_featured_image_url |
| includes/class-attachments.php | Returns inline image mappings with generic 'image/*' mime type |
| build/social-web/style-feed-stage.css | Compiled CSS missing featured-image styles due to component not being used |
| build/social-web/style-feed-stage-rtl.css | RTL version of compiled CSS, also missing featured-image styles |
| build/social-web/feed-stage.js | Compiled JavaScript with excerpt field changes but no featured-image field |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The featured image in the excerpt field now properly decodes HTML entities in the post title when used as alt text. This ensures that titles containing entities like &, ", etc. are properly displayed to screen readers and other assistive technologies.
Copilot Feedback ReviewI've reviewed all 7 Copilot comments on this PR. Here's the status: ✅ FixedHTML entities in alt text (excerpt/index.tsx:36)
❌ Not ApplicableThe remaining 6 comments reference the standalone Specific comments not applicable:
The new implementation displays the featured image within the excerpt field, maintaining aspect ratio with a 300px max dimension constraint. This provides a cleaner UI with the image positioned below the excerpt text. |
|
@jeherve @pfefferle Can you help me understand the need for this?
Why wouldn't I use the content view in that case? The more I use the app myself, the more I wonder if we should remove the excerpt view all-together. It looks pretty bad for Note content. |
|
I am quite the opposite. some full blog post breaks the readability a lot! |
|
Sure, but I wouldn't expect those to show up as Notes? And if they do that's probably something we should address with the platform they use. Maybe we should add that short form/long form switch sooner rather than later. |
|
Are we talking about the same thing? This is about having the excerpt in the main view not? What has this todo with the object-type filter? |
what is that? |
7f25182 to
0d86acc
Compare
Since the excerpt field was removed in the add/reader branch, this change migrates the featured image functionality into the content field instead. Changes: - Add featured image rendering to content field for both Notes (full content) and Articles (excerpt) - Decode HTML entities in alt text for accessibility - Add image styles for both content and excerpt contexts - Images display with 300px max dimensions and 4px border radius - Images have 1em top margin for proper spacing
Clean up the excerpt field directory that was left over from the migration to content field. The excerpt field was removed in the add/reader branch and its functionality has been merged into the content field.
obenland
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.
I still don't think this is needed. If you feel strongly about having it we should not process them and display them for Note types, though.
| // Save the featured image URL as post meta for efficient retrieval. | ||
| if ( ! empty( $files ) ) { | ||
| foreach ( $files as $file ) { | ||
| // Check if this is an image file. | ||
| if ( isset( $file['mime_type'] ) && 0 === strpos( $file['mime_type'], 'image/' ) ) { | ||
| \update_post_meta( | ||
| $post_id, | ||
| '_activitypub_featured_image_url', | ||
| $file['url'] | ||
| ); | ||
| break; // Only save the first image. | ||
| } | ||
| } | ||
| } |
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.
This should happen as part of Attachments::import_post_files()
| { hasFeaturedImage && ( | ||
| <img | ||
| src={ item.featured_image } | ||
| alt={ decodeEntities( item.title?.rendered || __( 'Post image', 'activitypub' ) ) } | ||
| className="activitypub-feed-content__image" | ||
| loading="lazy" | ||
| /> | ||
| ) } |
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.
We shouldn't add a featured image to Notes, it'll likely duplicate an image that gets displayed as part of the content.
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.
Yes, that was an unfinalized port from the excerpt/content system to the new "magic" content system.
Co-authored-by: Konstantin Obenland <[email protected]>
Co-authored-by: Konstantin Obenland <[email protected]>
Summary
Adds featured image display capability to the ActivityPub Social Web reader feed view.
Changes
_activitypub_featured_image_urlpost metafeatured_imagefield onap_postpost typefeaturedImageFieldcomponent for DataViewsfeatured_imagetoFeedPostinterface anduseFeedhook defaultsImplementation Details
Testing
Checklist