Skip to content

Conversation

fmendez
Copy link
Contributor

@fmendez fmendez commented May 9, 2013

No description provided.

fmendez added 3 commits May 9, 2013 17:44
count.

This endpoint is used to update the title to reflect
the correct unread stories count after clicking on a
given story.

[Fixes stringer-rss#5]
count.

This endpoint is used to update the title to reflect
the correct unread stories count after clicking on a
given story.

[Fixes stringer-rss#52]
@swanson
Copy link
Collaborator

swanson commented May 9, 2013

Thanks for this, looks like a good start. The title should just be "stringer | your rss buddy" when there are no unread items - let's drop the "(0)".

It seems kind of slow as well (maybe it is just in dev mode), but there is a noticeable lag if you are going through stories quickly. Maybe we can just keep the unread count locally on the clientside and subtract one?

@fmendez
Copy link
Contributor Author

fmendez commented May 9, 2013

The client-side approach sounds like a good idea, I'll be doing those changes then.

to the client side of the application.

[Fixes stringer-rss#52]
@swanson
Copy link
Collaborator

swanson commented May 10, 2013

Thanks - will review this when I get a chance.

@stefanoverna
Copy link

Why not dropping the endpoint altogether at this point? A data-unread-count attribute put somewhere in the DOM could easily do the same job, without cluttering the app routes.

@fmendez
Copy link
Contributor Author

fmendez commented May 10, 2013

@stefanoverna That's what I did in the last refactoring. The endpoint was removed and the logic was migrated to the client side.

@stefanoverna
Copy link

Cool, just noticed now. Sorry for the unnecessary noise @fmendez!

swanson pushed a commit that referenced this pull request May 11, 2013
Updating the title with the correct unread message count when clicking a story.
@swanson swanson merged commit f9b8afa into stringer-rss:master May 11, 2013
@swanson
Copy link
Collaborator

swanson commented May 11, 2013

Thanks!

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.

3 participants