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

Proposal: Pinned posts definitions #2455

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mary-ext
Copy link
Contributor

@mary-ext mary-ext commented Apr 30, 2024

This pull request proposes the lexicon definitions needed for pinned posts functionality.

The only "hard" part about implementing this is how pinned posts should be retrieved, the rest seems trivial, so I'm compelled to do whatever I can to get this shipped, and will probably ask for help for that bit.

User-facing

  • Up to two posts can be pinned at any given moment
    • I think this is a reasonable choice given Bluesky's 300-char limit, not to mention, this provides separation between what users might want to convey (e.g. about me in the first post, projects in the second)
  • Pinned posts doesn't have to be the user's own posts, but it might be worthwhile to restrict it for now

Technical details

  • Posts are pinned by attaching the post's URI to the pinnedPosts array in the profile record
    • should it be a strong ref? I think it should if we allow other user's posts to be pinned
    • if it were to be limited to 1, I think I'd still make this an array, more of just because though.
  • Pinned posts are retrieved by requesting filter: pinned in getAuthorFeed
  • You can tell if the post is pinned by checking the post's viewer state
    • will doing this check incur a noticeable increase in CPU time needed to hydrate a feed?
  • Hydrated profiles returns associated.pinnedPosts which provides indication for the amount of pinned posts that clients can expect
Archived

Problem

This is where the problem begins, how should the pinned posts be retrieved? I see two sides to this, and they're both valid.

  1. Including it as part of the author feed means that it's less work for clients to do (with no additional network request), and it would affect older clients as well (just without any indication of it being pinned)
  2. Being able to retrieve it separately (and subsequently the option to not include it in the feed response) means that clients gets to experiment with their own ideas as to how they'd like to display the pinned posts, without making assumptions (which I feel is critical)

This is incomplete, I'll write up my thoughts on this shortly with these two points in mind.

@yamarten
Copy link

I support that pinned posts should not be included in the author feed. If it works with an old client, it will make the user feel that it is inexplicable behavior.

For how to present it to clients, I prefer to keep profileView small. If I were to make it, I would choose to define a new record and get it with getRecord. On the other hand, the client author may want to avoid additional API calls.

associated is in the middle, but it is a tough choice for both. However, if there are few enough users of this function, I feel that this design is reasonable.

@mary-ext
Copy link
Contributor Author

mary-ext commented Apr 30, 2024

The additions to hydrated profile views are as minimal as it can be, it's a necessity if you want to retrieve pinned posts separately.

As an example, the Feeds and Lists tab on bsky.app's profile page. A while ago, those tabs would have a delay before being shown, this is because the hydrated profiles doesn't give any indication whether the user has them or not, so bsky.app had to retrieve it separately.

Same goes for moderation services, @moderation.bsky.app leads to the Labels tab instead of Posts specifically because the profile views tells the app that it does have a service record.

So the same has to go for pinned posts, say if you wanted to put the pinned posts in a separate tab, and have it default to showing that tab on opening the profile, then the app needs to know if the user even has pinned posts in the first place.

It needs to be on all views, whether it's profileView, profileViewBasic or profileViewDetailed

@dolciss
Copy link

dolciss commented Apr 30, 2024

I also +1 to not including it in the author feed because only the pinned posts are no longer in reverse chronological order.

I think one or two pinned posts is enough and it is reasonable to keep them in an array for future use.
For more than that, Bluesky can create a custom feed for the introductory post and direct you to it :)

@mary-ext
Copy link
Contributor Author

mary-ext commented May 1, 2024

okay I think it should be fine to have them separately, you'd get pinned posts if you request for filter: pinned on getAuthorFeed

Since associated.pinnedPosts returns the count it should be doable to skip making that additional network request if inclined.

@mary-ext mary-ext marked this pull request as ready for review May 1, 2024 13:23
@dolciss
Copy link

dolciss commented May 1, 2024

Thanks, I think making it a pinned_posts filter is a very good idea!

@mary-ext
Copy link
Contributor Author

mary-ext commented May 1, 2024

I have a semi-working implementation for this ready if we're going with this proposal as it is, just need help with how I should be retrieving the profile record for getPostViewerStates where the pinned viewer state should be set, since currently all it's passing is the DID of the viewer.

image

@mary-ext mary-ext mentioned this pull request May 2, 2024
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.

None yet

3 participants