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 pagination to addresses page #1812

Merged
merged 16 commits into from
May 6, 2019
Merged

add pagination to addresses page #1812

merged 16 commits into from
May 6, 2019

Conversation

ayrat555
Copy link
Contributor

@ayrat555 ayrat555 commented Apr 23, 2019

resolves #1782
fixes #1781

Changelog

  • add pagination to addresses page

@ghost ghost assigned ayrat555 Apr 23, 2019
@ghost ghost added the in progress label Apr 23, 2019
@coveralls
Copy link

coveralls commented Apr 23, 2019

Pull Request Test Coverage Report for Build 9f0f7269-a677-4bf2-b173-b2f7038573ca

  • 14 of 20 (70.0%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.04%) to 81.148%

Changes Missing Coverage Covered Lines Changed/Added Lines %
apps/block_scout_web/lib/block_scout_web/chain.ex 1 3 33.33%
apps/block_scout_web/lib/block_scout_web/controllers/address_controller.ex 7 11 63.64%
Files with Coverage Reduction New Missed Lines %
apps/indexer/lib/indexer/block/catchup/bound_interval_supervisor.ex 1 85.19%
Totals Coverage Status
Change from base Build a12d6bda-abc1-4152-bd2f-3716ef4e9e5a: 0.04%
Covered Lines: 4593
Relevant Lines: 5660

💛 - Coveralls

@ayrat555 ayrat555 changed the title [WIP] add pagination to addresses page add pagination to addresses page Apr 23, 2019
Copy link
Member

@vbaranov vbaranov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ayrat555

  1. I see this description:
    https://github.com/poanetwork/blockscout/blob/01356013ce4cd75c84e9fc7ee1103d7ba1f86bed/apps/explorer/lib/explorer/chain.ex#L1120

I think it is incompatible with your changes. Please, update it. And also please drop some comments in the PR: what is the new scheme of pagination.

  1. I can't understand from the pagination view on what page am I. Let's add (page {number_of_page}) like Showing 12 addresses of 93 total addresses with a balance (page 2) in the header.

  2. I have no ability to go back: there is no Back button.

  3. To switch the pagination view page I need to go to the bottom of the website page. It is not user-friendly. Let's repeat Back, Next buttons on the top of the list also.

  4. And something strange with switching between pages: https://www.loom.com/share/e7fb398054944dc79ec0b04fe843cbae

@vbaranov
Copy link
Member

@ayrat555 in the current design top view pagination could look like this:

Screenshot 2019-04-24 at 14 33 40

@ghost ghost assigned vbaranov Apr 25, 2019
@ayrat555 ayrat555 requested a review from vbaranov May 3, 2019 06:15
Copy link
Member

@vbaranov vbaranov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ayrat555

  1. Top buttons set should be aligned with the text on the left by the bottom edge (as in screenshot I attached before)

  2. I haven't seen the bottom buttons set. It should be there as before this PR.

@ayrat555
Copy link
Contributor Author

ayrat555 commented May 3, 2019

@vbaranov what do you mean by bottom buttons?
This is the master branch:
img

@vbaranov
Copy link
Member

vbaranov commented May 3, 2019

@ayrat555

ok, it is the best practice to put pagination buttons in the top and to the bottom of the list. In the upcoming design update of Blockscout there are both: bottom and top set of buttons for pagination.

@ayrat555
Copy link
Contributor Author

ayrat555 commented May 3, 2019

@vbaranov should I add to the top and to the bottom?

@vbaranov
Copy link
Member

vbaranov commented May 3, 2019

@vbaranov should I add to the top and to the bottom?

yes

@ayrat555
Copy link
Contributor Author

ayrat555 commented May 3, 2019

@vbaranov I addressed your comments. Can you please take a look?

@vbaranov vbaranov self-requested a review May 6, 2019 09:48
@vbaranov vbaranov merged commit cb894aa into master May 6, 2019
@ghost ghost removed the in progress label May 6, 2019
@vbaranov vbaranov deleted the ab-paginate-addresses branch May 6, 2019 09:57
@vbaranov vbaranov mentioned this pull request May 6, 2019
3 tasks
vbaranov added a commit that referenced this pull request May 20, 2019
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.

Add pagination for all accounts page Wrong number of accounts in accounts page
5 participants