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

Channel page allows navigation to pages beyond content limit #2059

Closed
10 tasks
jessopb opened this issue Oct 26, 2018 · 6 comments
Closed
10 tasks

Channel page allows navigation to pages beyond content limit #2059

jessopb opened this issue Oct 26, 2018 · 6 comments
Assignees
Labels
level: 2 Some knowledge of the existing code is recommended type: bug Existing functionality is wrong or broken

Comments

@jessopb
Copy link
Member

jessopb commented Oct 26, 2018

The Issue

A Channel page presents navigation to pages beyond content limit

Steps to Reproduce

  1. Go to a channel page, such as 3Blue1Brown.
  2. Click to go to page 3
  3. -Whitescreen-

Expected Behaviour

With 48 items per page, a channel with less than 96 items should not show an option to go to page 3 or higher.

Actual Behaviour

Channel page UI presents nav options up to page 8.

System Configuration

App | 0.25.1
Daemon (lbrynet) | 0.21.2
Wallet (lbryum) | 3.2.4
Platform | Linux (Linux-4.14.69-1-MANJARO-x86_64-with-ManjaroLinux-17.1.12-Hakoila)
Installation ID | ADdEbSfxc5sjkUMbgBugDH4Dk8C2sULhd5XyphXqq1aVrMzrJKTRvVWbfjR6YVDqyT

Internal Use

Acceptance Criteria

Definition of Done

  • Tested against acceptance criteria
  • Tested against the assumptions of the user story
  • The project builds without errors
  • Unit tests are written and passing
  • Tests on devices/browsers listed in the issue have passed
  • QA performed & issues resolved
  • Refactoring completed
  • Any configuration or build changes documented
  • Documentation updated
  • Peer Code Review performed
@jessopb
Copy link
Member Author

jessopb commented Oct 26, 2018

Is it wise to have
export const CLAIMS_PER_PAGE = 48 for this value?
LBRY-DESKTOP:
Lbry.claim_list_by_channel({ uri, page: page || 1, page_size: 48 }).then(result => {
LBRY-REDUX:

export const makeSelectTotalPagesForChannel = uri =>
  createSelector(
    selectChannelClaimCounts,
    byUri => byUri && byUri[uri] && Math.ceil(byUri[uri] / 10)
  );

@neb-b
Copy link

neb-b commented Oct 26, 2018

@jessopb Good find. Yeah, that would have prevented this bug in the first place.

@jessopb
Copy link
Member Author

jessopb commented Oct 26, 2018

Is that a potential customizeable setting for people with different screen sizes or more or less computing power?

@neb-b
Copy link

neb-b commented Oct 26, 2018

Possibly in the future.

I think for now makeSelectTotalPagesForChannel should take an additional argument page_size (which will be a new constant) that would default to 10. Then the android app wouldn't have to make any changes for this update.

@alyssaoc alyssaoc added the type: bug Existing functionality is wrong or broken label Oct 29, 2018
@neb-b
Copy link

neb-b commented Oct 30, 2018

@jessopb Were you planning on working on this?

@neb-b neb-b added the level: 2 Some knowledge of the existing code is recommended label Oct 30, 2018
@alyssaoc alyssaoc added this to the Nov 12th (Desktop) milestone Oct 30, 2018
@jessopb
Copy link
Member Author

jessopb commented Oct 30, 2018

It's up for grabs. I'll get to it November 1st otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
level: 2 Some knowledge of the existing code is recommended type: bug Existing functionality is wrong or broken
Projects
None yet
Development

No branches or pull requests

3 participants