-
Notifications
You must be signed in to change notification settings - Fork 8
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
v1 of Bluesky embed support #304
base: main
Are you sure you want to change the base?
Conversation
|
Wowowow you've been busy! Again, I need a few days to fully review but I'm excited to merge this. Thanks for your patience and especially your hard work! |
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.
First off: this is heroic work, and thank you so much. Bluesky has definitely been on my mind and I'm really happy to add it to the family.
Secondly — this is annoying, and I'm sorry in advance: the file structure you've created here is based on an older generation of the plugins, which I've been (slowly) going through and refactoring. The later ones look more like packages/ted
.
You can see a more thorough breakdown in the Contribution guide. The advantage of the newer structure is that it uses just one RegEx (pattern.js
), instead of two slight variations (spotPattern.js
and extractMatch.js
). More consistent, easier to test, more performant, etc.
I haven't gone through and done a line-by-line review here — I thought I'd flag this and see how you want to handle it first. It is a fairly major refactor to ask for, I know, so don't feel obliged. But I'd love to add this functionality!
Ah, sorry about that. Was trying to match the Twitter behavior and should have looked at all the packages. Can give it a look tomorrow. |
Got a wild hair and decided to work on it tonight. Please test and see it it works. All tests are passing. Realized first time around I did not reference it elsewhere in the root of the project. Tried to model off of the TED version as much as possible. Left room for custom Bluesky servers in the future, though should probably be handled slightly differently (ie: is this going to be like Mastodon and we have to support a list of them or can look for some other identifier in the HEAD etc.) I did not submit a changeset but I did put in a CHANGELOG. Feel free to change anything about this. I just want my Bsky unfurls. ;) |
All the plugin code looks in really great shape to me, no real notes to add! It produces exactly the HTML output I'd expect and everything seems solid. But when I try to demo it (I pushed a commit to add this — run ![]() Just some stray notes and threads to pull on:
Happy to chat further as we debug, and I'll do some more investigation too. Would love to get this out! |
* | ||
* Notable points: | ||
* - Matches both bsky.app and staging.bsky.app domains | ||
* - Post IDs are alphanumeric and at least 10 characters long |
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.
Would there be a logical maximum character count for the post ID? Just wondering about RegEx efficiency 🤔
Hmm… I have some suspicions and am confident I can resolve this because I
have something similar running already, but I didn’t do enough testing in a
logged out of Bsky browser so will see what’s going on tonight.
…On Tue, Feb 4, 2025 at 3:51 PM Graham F. Scott ***@***.***> wrote:
All the plugin code looks in really great shape to me, no real notes to
add! It produces exactly the HTML output I'd expect and everything seems
solid. But when I try to demo it (I pushed a commit
<e053822>
to add this — run pnpm dev from the project root to see it) the frame
doesn't load for me. Devtools show an iframe permissions issue:
Screenshot.2025-02-04.at.18.31.02.png (view on web)
<https://github.com/user-attachments/assets/8b68b343-82db-4814-a302-303aa92d26fb>
Just some stray notes and threads to pull on:
- I thought it might be an insecure http/localhost issue, but saw the
same thing with an https tunnel.
- If I click through the URL in the iframe's src attribute (where it's
the post URL with /embed appended), that throws a 404 for me and then
just redirects to bsky.app.
- The official Bluesky embed service produces an HTML Blockquote and a
JS script — not the iframe in your implementation. It's possible they're
blocking the iframe behavior deliberately but I haven't investigated
further. If that's the case there's definitely a way forward (see the
Twitter and Instagram plugins), but it gets more complex than the simple
URL-to-iframe pipeline.
Happy to chat further as we debug, I'll do some more investigation. Would
love to get this out!
—
Reply to this email directly, view it on GitHub
<#304 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAZ5NISDEXTS23E3K3PC7L2OFG75AVCNFSM6AAAAABWIY54J6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMZVGM2DENJVHA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Add Bluesky Embed Support
This PR adds support for embedding Bluesky posts in Eleventy sites, following the same pattern as other social media embeds in this plugin collection.
Features
bsky.app/profile/...
)Implementation Details
https://bsky.app/oembed
Testing
Documentation
Closes #301 (if there's an existing issue)