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 go to page navigation to channel pagination options #1166

Merged
merged 2 commits into from Mar 30, 2018

Conversation

Projects
None yet
3 participants
@miikkatu
Contributor

miikkatu commented Mar 22, 2018

This would close #711.

The component used for pagination (react-navigate) doesn't offer customisation options, so a separate control for choosing the target page is needed. The implementation is such that the user would enter a page number and presses the enter key.

unfocused

focused

@liamcardenas

This comment has been minimized.

Show comment
Hide comment
@liamcardenas

liamcardenas Mar 29, 2018

Contributor

@seanyesmunt assigning this code review to you based on what we discussed in slack.

Contributor

liamcardenas commented Mar 29, 2018

@seanyesmunt assigning this code review to you based on what we discussed in slack.

@liamcardenas liamcardenas requested a review from seanyesmunt Mar 29, 2018

@liamcardenas liamcardenas removed their request for review Mar 29, 2018

@seanyesmunt

This comment has been minimized.

Show comment
Hide comment
@seanyesmunt

seanyesmunt Mar 29, 2018

Member

@miikkatu Thanks for the contribution!!

You should be able to use <FormRow verticallyCenterered> ... </FormRow> and <FormField type="text" prefix={__('Go to page:')} />

Instead of adding more css. I think the only thing you might need to do is add a class input__paginate or something to set a width on the input so it isn't too long. And a new prop to FormRow, maybe centered? To add justify-content: center

Something like this:

            <FormRow verticallyCentered centered>
              <ReactPaginate
                pageCount={totalPages}
                pageRangeDisplayed={2}
                previousLabel="‹"
                nextLabel="›"
                activeClassName="pagination__item--selected"
                pageClassName="pagination__item"
                previousClassName="pagination__item pagination__item--previous"
                nextClassName="pagination__item pagination__item--next"
                breakClassName="pagination__item pagination__item--break"
                marginPagesDisplayed={2}
                onPageChange={e => this.changePage(e.selected + 1)}
                initialPage={parseInt(page - 1, 10)}
                containerClassName="pagination"
              />

              <FormField
                type="text"
                id="paginate-page"
                prefix={__('Go to page:')}
                onKeyUp={e => this.paginate(e, totalPages)}
              />
          </FormRow>
Member

seanyesmunt commented Mar 29, 2018

@miikkatu Thanks for the contribution!!

You should be able to use <FormRow verticallyCenterered> ... </FormRow> and <FormField type="text" prefix={__('Go to page:')} />

Instead of adding more css. I think the only thing you might need to do is add a class input__paginate or something to set a width on the input so it isn't too long. And a new prop to FormRow, maybe centered? To add justify-content: center

Something like this:

            <FormRow verticallyCentered centered>
              <ReactPaginate
                pageCount={totalPages}
                pageRangeDisplayed={2}
                previousLabel="‹"
                nextLabel="›"
                activeClassName="pagination__item--selected"
                pageClassName="pagination__item"
                previousClassName="pagination__item pagination__item--previous"
                nextClassName="pagination__item pagination__item--next"
                breakClassName="pagination__item pagination__item--break"
                marginPagesDisplayed={2}
                onPageChange={e => this.changePage(e.selected + 1)}
                initialPage={parseInt(page - 1, 10)}
                containerClassName="pagination"
              />

              <FormField
                type="text"
                id="paginate-page"
                prefix={__('Go to page:')}
                onKeyUp={e => this.paginate(e, totalPages)}
              />
          </FormRow>
@seanyesmunt

This comment has been minimized.

Show comment
Hide comment
@seanyesmunt

seanyesmunt Mar 29, 2018

Member

Please feel free to reach out on discord if you have any questions!

Member

seanyesmunt commented Mar 29, 2018

Please feel free to reach out on discord if you have any questions!

@seanyesmunt

Almost there. I would like to avoid adding a lot of CSS for this

@miikkatu

This comment has been minimized.

Show comment
Hide comment
@miikkatu

miikkatu Mar 30, 2018

Contributor

I updated the PR. I agree that the suggested solution is definitely better.

I also fixed some unrelated issues in the same file that prevented committing, because of the new rules introduced after the initial PR.

The new version looks like this:

screen shot 2018-03-30 at 19 34 20

Contributor

miikkatu commented Mar 30, 2018

I updated the PR. I agree that the suggested solution is definitely better.

I also fixed some unrelated issues in the same file that prevented committing, because of the new rules introduced after the initial PR.

The new version looks like this:

screen shot 2018-03-30 at 19 34 20

@seanyesmunt

This comment has been minimized.

Show comment
Hide comment
@seanyesmunt

seanyesmunt Mar 30, 2018

Member

@miikkatu Awesome! Looking a lot better. Could you just change the paginate-channel id to a class?

Member

seanyesmunt commented Mar 30, 2018

@miikkatu Awesome! Looking a lot better. Could you just change the paginate-channel id to a class?

@miikkatu

This comment has been minimized.

Show comment
Hide comment
@miikkatu

miikkatu Mar 30, 2018

Contributor

That id is now a class, done!

Contributor

miikkatu commented Mar 30, 2018

That id is now a class, done!

@seanyesmunt

This comment has been minimized.

Show comment
Hide comment
@seanyesmunt

seanyesmunt Mar 30, 2018

Member

Sweet! I'll merge once the tests pass. @tzarebczan Will reach out with LBC payment

Member

seanyesmunt commented Mar 30, 2018

Sweet! I'll merge once the tests pass. @tzarebczan Will reach out with LBC payment

@seanyesmunt seanyesmunt merged commit d6c7953 into lbryio:master Mar 30, 2018

2 checks passed

Codacy/PR Quality Review Good work! A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment