Skip to content
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

fix rendering of external in quote embeds #2464

Merged

Conversation

haileyok
Copy link
Contributor

External embeds are not currently displayed in a quote post, which results in context loss.

I also think it makes more sense here to remove the .find() since - unless this is done in anticipation of future changes - there will never be more than a single embed for a post: https://github.com/bluesky-social/atproto/blob/50f70453a9e49621956e4e820226c0e220fe138e/packages/bsky/src/services/feed/views.ts#L434 (thanks @mary-ext). Instead we just need to get the first possible embed from the array.

We do still need to check for types, since we don't want to show a quote's quote.

RocketSim_Recording_iPhone_15_Pro_6 1_2024-01-09_18 36 15

Of course we could add

  const externalEmbed = React.useMemo(
    () =>
      quote.embeds?.find(
        embed =>
          AppBskyEmbedExternal.isView(embed)
      ),
    [quote.embeds],
  )

but this feels like some additional logic that isn't needed. Correct me if wrong.

@pfrazee
Copy link
Collaborator

pfrazee commented Jan 11, 2024

Strong +1 to this. It was the wrong call not to do link cards.

I actually don't recall why app.bsky.embed.record#viewRecord uses an array of embeds but I assume it was for future defensibility.

@pfrazee pfrazee merged commit 6ec6d52 into bluesky-social:main Jan 11, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants