(Feature) New pagination view #1928

gabitoesmiapodo · 2019-05-10T19:43:53Z

Closes #1893

Motivation

Pagination styles need to be updated according to this design:

This is obviously missing some logic which the design shows but it's absent from the current implementation:

First and Last buttons.
Page 1 of X needs to calculate the value for X.
Show XXXX Records, amount of items to show should be implemented too (this can be hidden using a parameter).

Sections including the pagination component:

Blocks / Uncles / Forked.
Transactions Validated / Pending.
Accounts.

Screenshots

(Colors will obviously be different depending on your current theme)

Changelog

Enhancements

Pagination styles were updated.

Checklist for your PR

I added an entry to CHANGELOG.md with this PR
If I added new functionality, I added tests covering it.
If I fixed a bug, I added a regression test to prevent the bug from silently reappearing again.
I checked whether I should update the docs and did so if necessary

coveralls · 2019-05-10T19:57:12Z

Pull Request Test Coverage Report for Build d3280b17-3fe5-4fcf-b9d2-c87162fd1601

0 of 0 changed or added relevant lines in 0 files are covered.
6 unchanged lines in 3 files lost coverage.
Overall coverage decreased (-0.07%) to 81.753%

Files with Coverage Reduction	New Missed Lines	%
apps/indexer/lib/indexer/fetcher/token.ex	1	78.57%
apps/indexer/lib/indexer/fetcher/token_balance.ex	2	87.1%
apps/indexer/lib/indexer/block/fetcher.ex	3	86.57%

Totals
Change from base Build f74b48aa-2a4d-4b9b-a26a-7397911f0ef5:	-0.07%
Covered Lines:	4646
Relevant Lines:	5683

💛 - Coveralls

ayrat555 · 2019-05-13T08:39:44Z

Page 1 of undefined is it ok?

Also, if there are no blocks should we show navigation buttons?

ayrat555 · 2019-05-13T08:41:27Z

...ock_scout_web/lib/block_scout_web/templates/common_components/_pagination_container.html.eex

+                </svg>
+            </a>
+        </li>
+        <li class="page-item"><a class="page-link no-hover" href><%= gettext "Page" %> <%= assigns[:cur_page_number] || "" %> <%= gettext "of" %> <%= assigns[:total_pages_number] || "undefined" %></a></li>


maybe we should show only current page if there is no total_pages_number instead of of undefined

Done.

I think everything else will be addressed in #1930

vbaranov

@gabitoesmiapodo

The pagination view looks good to me. Meanwhile, I guess, there are some changes should be done before merging this:

The old pagination messages like this Showing 10 Validated Transactions or Showing 50 addresses of 0 total addresses with a balance (page 1) should be removed as it is implemented in the new pagination view.
Could we display pagination buttons in the mobile view aligned to the center of the screen (see the screenshot below)?
As you correctly mentioned there are some new buttons and missing logic for new pagination. And I created a separate issue to fill those missing parts of the logic Implement logic for new advanced pagination functionality #1930. In the meantime, the logic for left/right pagination buttons exists in the current pagination, but it wasn't ported to the new pagination buttons. Could you return it? Because for now, pagination is rather a mockup.

vbaranov · 2019-05-13T09:57:53Z

@gabitoesmiapodo

In addition, we have pagination buttons for transactions list on address details page like this: https://blockscout.com/poa/core/address/0xc76815f3815614f10b78dfe94185e86d9f5250c5/transactions. We should also use new pagination here.

vbaranov · 2019-05-13T11:03:47Z

@gabitoesmiapodo

plus, we should have the same pagination for transactions in the block details page. For example, here https://blockscout.com/eth/mainnet/blocks/7751624/transactions

gabitoesmiapodo · 2019-05-13T11:04:30Z

@vbaranov

As you correctly mentioned there are some new buttons and missing logic for new pagination. And I created a separate issue to fill those missing parts of the logic #1930. In the meantime, the logic for left/right pagination buttons exists in the current pagination, but it wasn't ported to the new pagination buttons. Could you return it? Because for now, pagination is rather a mockup.

I didn't get that part, could you please clarify?

Working on fixing everything else.

vbaranov · 2019-05-13T11:13:06Z

@gabitoesmiapodo

As you correctly mentioned there are some new buttons and missing logic for new pagination. And I created a separate issue to fill those missing parts of the logic #1930. In the meantime, the logic for left/right pagination buttons exists in the current pagination, but it wasn't ported to the new pagination buttons. Could you return it? Because for now, pagination is rather a mockup.

I didn't get that part, could you please clarify?

Sure, I mean, in the current pagination (that we would like to update) we already have implemented left|right buttons and logic for them:

And that logic has been removed from the new left|right buttons (that are from this PR). Could we return that logic to those buttons before merging?

gabitoesmiapodo · 2019-05-13T11:16:50Z

@vbaranov

Oh, sure.

I thought that I've left that working, but it seems it's not.

* master: Update CHANGELOG.md Update sokol_logo.svg Update kovan_logo.svg Update _sokol_variables.scss Update _kovan_variables.scss # Conflicts: # CHANGELOG.md

vbaranov · 2019-05-13T14:37:25Z

@gabitoesmiapodo

in addition, the same pagination should be on the address details page for

coin balances
tokens
internal transactions

* master: Update CHANGELOG.md Update _modal.scss Update _button.scss # Conflicts: # CHANGELOG.md

gabitoesmiapodo · 2019-05-13T21:57:16Z

@vbaranov

The old pagination messages like this Showing 10 Validated Transactions or Showing 50 addresses of 0 total addresses with a balance (page 1) should be removed as it is implemented in the new pagination view.

Removed all text.

Could we display pagination buttons in the mobile view aligned to the center of the screen (see the screenshot below)?

Done.

As you correctly mentioned there are some new buttons and missing logic for new pagination. And I created a separate issue to fill those missing parts of the logic #1930. In the meantime, the logic for left/right pagination buttons exists in the current pagination, but it wasn't ported to the new pagination buttons. Could you return it? Because for now, pagination is rather a mockup.

in addition, the same pagination should be on the address details page for

I've only found the combination Back / Next on the Accounts section. Seems to be working now, but it is disabled if there's no back or next page.

I've replaced the pagination in every other section too, but in those cases there was only an Older or Next button. In that case, it's going to be replaced by the > button until its implementation is updated.

vbaranov

@gabitoesmiapodo

Could we display pagination buttons in the mobile view aligned to the center of the screen (see the screenshot below)?

Done.

Could we change the paddings for <> buttons to nail them to the left and right sides as shown on the screenshot from my previous review? This request is only for the mobile view.

I've replaced the pagination in every other section too, but in those cases there was only an Older or Next button. In that case, it's going to be replaced by the > button until its implementation is updated.

I see now that the pagination buttons work for Accounts page, but could we also return the logic to the > button, which is instead of Older or Next button?

* master: feat: add on demand fetching and stale attr to rpc Add CHANGELOG entry check presence of overlap[i]

gabitoesmiapodo · 2019-05-14T12:11:05Z

@vbaranov

Could we change the paddings for...

Sure, done. It should look like this:

I see now that the pagination buttons work for Accounts page, but could we also return the logic to the > button, which is instead of Older or Next button?

Not sure if I follow.

For the "Accounts" section there were only Back and Next buttons. Those were converted to < and > now.

For the rest of the pages I only found one Older (or Next) isolated button, which is now the > button.

These buttons are disabled when there's nothing to paginate (@prev_page_path / @next_page_path are null, etc.), but they could be hidden instead.

So, the functionality should be there (unless I screwed something up, which could be the case).

Maybe I inadvertently removed some other functionality?

vbaranov · 2019-05-14T12:49:19Z

@gabitoesmiapodo

I am trying to open blocks page, transactions page or other pages with pagination from your PR. On those pages, we have currently in the prod Older or Next buttons, and they work fine in the prod. When I open those pages from this PR, > button is always disabled. Please, try it locally.

gabitoesmiapodo · 2019-05-14T13:30:32Z

@vbaranov

Got it now, thank you.

It should work as before now, warts and all. Notice that the button will be disabled once there are no more pages, and there's no current way to go back.

vbaranov · 2019-05-14T15:01:32Z

@gabitoesmiapodo yes, seems, pagination buttons work now. The last thing I forgot to mention in the previous review is:

we should move the top pagination below the chart because the cart is not responsive on changing pages. It shows the whole data.

gabitoesmiapodo · 2019-05-14T15:25:25Z

@vbaranov

Done.

gabitoesmiapodo added 2 commits May 10, 2019 16:15

(add) new pagination component for accounts section

4635c15

(add) pagination component to transactions and blocks sections.

cee0d68

gabitoesmiapodo requested review from vbaranov, ayrat555 and saneery May 10, 2019 19:43

gabitoesmiapodo self-assigned this May 10, 2019

ghost added the in progress label May 10, 2019

gabitoesmiapodo added 3 commits May 10, 2019 16:44

(add) internationalization for pagination

c09b6fe

(fix) syntax

f091d6f

(update) changelog

2155ec4

Merge branch 'master' into feature/#1893-new-pagination-view

8e04dda

ghost assigned vbaranov May 12, 2019

vbaranov mentioned this pull request May 12, 2019

Implement logic for new advanced pagination functionality #1930

Open

Merge branch 'master' into feature/#1893-new-pagination-view

f9a3f1f

ayrat555 reviewed May 13, 2019

View reviewed changes

vbaranov reviewed May 13, 2019

View reviewed changes

Merge branch 'master' into feature/#1893-new-pagination-view

0aeece6

* master: Update CHANGELOG.md Update sokol_logo.svg Update kovan_logo.svg Update _sokol_variables.scss Update _kovan_variables.scss # Conflicts: # CHANGELOG.md

gabitoesmiapodo added 3 commits May 13, 2019 18:54

(update) all pagination buttons in all sections

4d333fc

Merge branch 'master' into feature/#1893-new-pagination-view

0c8fd39

* master: Update CHANGELOG.md Update _modal.scss Update _button.scss # Conflicts: # CHANGELOG.md

(update) internationalization files

e2bc4ee

gabitoesmiapodo requested a review from ayrat555 May 13, 2019 21:58

gabitoesmiapodo requested review from vbaranov and ayrat555 and removed request for ayrat555 May 13, 2019 21:58

vbaranov reviewed May 14, 2019

View reviewed changes

gabitoesmiapodo added 2 commits May 14, 2019 09:02

(fix) pagination alignment

2856ce6

Merge branch 'master' into feature/#1893-new-pagination-view

be3b02a

* master: feat: add on demand fetching and stale attr to rpc Add CHANGELOG entry check presence of overlap[i]

Merge branch 'master' into feature/#1893-new-pagination-view

bad6f5f

gabitoesmiapodo added 2 commits May 14, 2019 10:16

(fix) "next / older" pagination button

de46601

(fix) little refactoring

101fdc3

(fix) balance section pagination position

9b9f464

vbaranov self-requested a review May 14, 2019 15:40

vbaranov approved these changes May 14, 2019

View reviewed changes

Fix gettext test

38b5886

vbaranov merged commit 93c67bf into master May 14, 2019

vbaranov deleted the feature/#1893-new-pagination-view branch May 14, 2019 15:51

ghost removed the in progress label May 14, 2019

vbaranov mentioned this pull request May 17, 2019

Missing new pagination on some pages #1972

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(Feature) New pagination view #1928

(Feature) New pagination view #1928

gabitoesmiapodo commented May 10, 2019

coveralls commented May 10, 2019 •

edited

Loading

ayrat555 commented May 13, 2019

ayrat555 May 13, 2019

gabitoesmiapodo May 13, 2019

vbaranov left a comment

vbaranov commented May 13, 2019

vbaranov commented May 13, 2019

gabitoesmiapodo commented May 13, 2019

vbaranov commented May 13, 2019

gabitoesmiapodo commented May 13, 2019

vbaranov commented May 13, 2019

gabitoesmiapodo commented May 13, 2019 •

edited

Loading

vbaranov left a comment

gabitoesmiapodo commented May 14, 2019

vbaranov commented May 14, 2019

gabitoesmiapodo commented May 14, 2019

vbaranov commented May 14, 2019

gabitoesmiapodo commented May 14, 2019

(Feature) New pagination view #1928

(Feature) New pagination view #1928

Conversation

gabitoesmiapodo commented May 10, 2019

Motivation

Screenshots

Changelog

Enhancements

Checklist for your PR

coveralls commented May 10, 2019 • edited Loading

Pull Request Test Coverage Report for Build d3280b17-3fe5-4fcf-b9d2-c87162fd1601

💛 - Coveralls

ayrat555 commented May 13, 2019

ayrat555 May 13, 2019

Choose a reason for hiding this comment

gabitoesmiapodo May 13, 2019

Choose a reason for hiding this comment

vbaranov left a comment

Choose a reason for hiding this comment

vbaranov commented May 13, 2019

vbaranov commented May 13, 2019

gabitoesmiapodo commented May 13, 2019

vbaranov commented May 13, 2019

gabitoesmiapodo commented May 13, 2019

vbaranov commented May 13, 2019

gabitoesmiapodo commented May 13, 2019 • edited Loading

vbaranov left a comment

Choose a reason for hiding this comment

gabitoesmiapodo commented May 14, 2019

vbaranov commented May 14, 2019

gabitoesmiapodo commented May 14, 2019

vbaranov commented May 14, 2019

gabitoesmiapodo commented May 14, 2019

coveralls commented May 10, 2019 •

edited

Loading

gabitoesmiapodo commented May 13, 2019 •

edited

Loading