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

Add timeline read markers API #11762

Merged
merged 1 commit into from Sep 6, 2019

Conversation

@Gargron
Copy link
Member

commented Sep 4, 2019

Fix #4093, fix #7418, close #6756

Method Description
GET /api/v1/markers Get markers specified by timeline array param. Response is a map with timeline names as keys
POST /api/v1/markers Create new marker(s) with last_read_id by submitting a map with timeline names as keys. The keys can be home or notifications

JSON returned includes last_read_id, updated_at and version (which increments on every update).

@Gargron Gargron added the api label Sep 4, 2019

@Gargron Gargron force-pushed the feature-read-markers branch from d625f50 to 34d7282 Sep 5, 2019

app/javascript/mastodon/actions/markers.js Outdated Show resolved Hide resolved
app/javascript/mastodon/actions/markers.js Outdated Show resolved Hide resolved

@Gargron Gargron force-pushed the feature-read-markers branch from 34d7282 to 5d983b2 Sep 5, 2019

@ykzts
ykzts approved these changes Sep 5, 2019

@Gargron Gargron merged commit e445a8a into master Sep 6, 2019

2 checks passed

build-and-test Workflow: build-and-test
Details
codeclimate All good!
Details

@Gargron Gargron deleted the feature-read-markers branch Sep 6, 2019

@nolanlawson

This comment has been minimized.

Copy link
Collaborator

commented Sep 8, 2019

I have a few questions about this implementation:

  1. It seems the Mastodon webapp will submit the markers in the unload event. So if a user keeps their tab open in their browser and never closes it, another client will never know what the last read position is? It seems like it may be better to do this lazily (e.g. every few minutes), as well as during the unload event and when a page goes into a background tab.
  2. This is only for the home/notifications timelines right now, if I understand correctly?
  3. Is the assumption that, on first load, a client will immediately scroll to the saved position in the timeline if a marker is found? So if a user has not checked Mastodon in several days and there are 100s of statuses, should the app load all 100+ of them and then scroll to the position? Or give up and reset the position? Or lazily show the new content?

/cc @charlag

@charlag

This comment has been minimized.

Copy link

commented Sep 8, 2019

I agree with @nolanlawson on the first one.
I think when you open it on another client you just load:

  1. The latest ones
  2. Statuses with max_id = marker and
    and put a loadable gap between them (I think we do something like that already).
    cc @connyduck
@Gargron

This comment has been minimized.

Copy link
Member Author

commented Sep 8, 2019

It seems like it may be better to do this lazily (e.g. every few minutes), as well as during the unload event and when a page goes into a background tab.

Well, what if you have multiple clients open? And they all try to update the position at the same time? If they all use the streaming API I suppose they would arrive at similar or identical results but it seems wasteful to just keep updating it back and forth between them all. I'm not sure what the point would be of saving and loading a marker while a client is already actively open, the other app would then load the top of the timeline anyway..?

@charlag

This comment has been minimized.

Copy link

commented Sep 8, 2019

It could be done in the way that we send it to the server and server writes it only when the client stops communicating but that's complicated I guess.
I don't think anyone will load marker when the app is already open, only on startup. If two clients are open, the last used one is likely to win (which makes sense to me).

Edit: a lot of spelling

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.