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

Reader: tweak to combined card #12097

Closed
wants to merge 2 commits into from
Closed

Conversation

samouri
Copy link
Contributor

@samouri samouri commented Mar 14, 2017

This PR aims to experiment with CombinedCards. One of the goals of combined cards is to make it so that a noisy site cannot overwhelm your following stream. This PR takes that goal and runs with it wayy further

This PR changes the current behavior of CCs in two ways:

  • lifts the requirement that combined posts must be consecutive. Instead each site has a maximum number of CARDS_PER_DAY (combined cards counts as 1). As of now its set to 4. Real Example: Lets say you follow uproxx which posts a ton of times per day. Now it will at-maximum be allowed to have 4 separate combined cards throughout the day
  • combined cards will group together as many posts that occurred during the allotted timeframe (6 hours currently because 4 cards per day and 24/4 = 6). It will max out at displaying 5.

known brokeness:

  • keyboard shortcuts. they currently rely on the original postKey ordering whereas the entire point of this PR is to see how it feels if we break with reverse chronological

After making this PR it made me think of the following stream a bit differently. Instead of being a "complete collection of everything I follow in reverse-chronological order", it become more like a jumping off point to explore the things that I follow.

cc @fraying

edit: the code is currently super messy, so feel free not to look at it 😟

samouri added 2 commits March 14, 2017 03:09
1. only show max N cards per site per day
2. only display Y cards within a combined card. make the rest force you to visit the site-stream
@samouri samouri self-assigned this Mar 14, 2017
@matticbot
Copy link
Contributor

@matticbot matticbot added the [Size] L Large sized issue label Mar 14, 2017
@samouri
Copy link
Contributor Author

samouri commented Mar 14, 2017

Closing branch because of special character ruining calypso.live

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Reader The reader site on Calypso. [Size] L Large sized issue [Type] Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants