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.

Sign up for GitHub

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

Jump to bottom

Timebased blocks listing #779

Merged
merged 5 commits into from
Nov 2, 2018
Merged

Timebased blocks listing #779

merged 5 commits into from
Nov 2, 2018

Conversation

dmigwi
Copy link
Member

@dmigwi dmigwi commented Oct 30, 2018

PR introduce time-based blocks list view pages below. Grouping is done using the block time value.
"/years" - Groups blocks into years
"/months" - groups blocks into months
"/weeks' - Groups blocks into weeks
"/days' - Groups blocks into days

All the above pages follow this similar format.
screenshot from 2018-10-30 16-26-23

@dmigwi dmigwi force-pushed the timebased-blocks-listing branch 2 times, most recently from ed82945 to 846da88 Compare October 30, 2018 13:43
chappjc
chappjc reviewed Oct 30, 2018
View reviewed changes
db/dbtypes/types.go Outdated Show resolved Hide resolved
chappjc
chappjc requested changes Oct 30, 2018
View reviewed changes
explorer/explorer.go Outdated Show resolved Hide resolved
explorer/explorer.go Outdated Show resolved Hide resolved
db/dcrpg/pgblockchain.go Show resolved Hide resolved
db/dcrpg/queries.go Show resolved Hide resolved
db/dcrpg/queries.go Outdated Show resolved Hide resolved
db/dcrpg/queries.go Show resolved Hide resolved
explorer/explorer.go Outdated Show resolved Hide resolved
explorer/explorerroutes.go
"Time based blocks listing page cannot run in lite mode.", NotSupportedStatusType)
}

grouping := dbtypes.TimeGroupingFromStr(val)
Copy link
Member

Choose a reason for hiding this comment

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

Please pass a TimeBasedGrouping instead of a string to timeBasedBlocksListing. This should be simpler, and will eliminate the potential future bug where TimeGroupingFromStr and the strings in the functions like YearBlocksListing may no longer agree.

Copy link
Member Author

Choose a reason for hiding this comment

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

The string is needed on the front end since the four routes share a template. I could pass the string and the TimeBasedGrouping object but that might result to duplication and also create bugs when there are inconsistencies between the two values set.

Copy link
Member

Choose a reason for hiding this comment

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

The nice thing about objects with methods is that you can call the method from the template. :D I'm referring to func (g TimeBasedGrouping) String() string. In fact, since this type implements the Stringer interface, I'd be surprised if you even need to call String() manually, but you can.

Copy link
Member

Choose a reason for hiding this comment

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

This can be revisited, but please note by above comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

This function func (g TimeBasedGrouping) String() string is going to be very important in relation to the next PR that I am currently testing.
Comment noted though.

@dmigwi dmigwi force-pushed the timebased-blocks-listing branch 2 times, most recently from 08fa6a4 to 2cb6aba Compare October 30, 2018 20:30
@dmigwi
Copy link
Member Author

dmigwi commented Oct 30, 2018 •
edited
Loading

Latest Update after dropping the mempool implementation.

screenshot from 2018-10-31 01-34-20

chappjc
chappjc requested changes Oct 31, 2018
View reviewed changes
explorer/templates.go Outdated Show resolved Hide resolved
explorer/templates.go Outdated Show resolved Hide resolved
explorer/templates.go Outdated Show resolved Hide resolved
@chappjc chappjc added this to the 3.2 milestone Oct 31, 2018
@dmigwi dmigwi force-pushed the timebased-blocks-listing branch 2 times, most recently from 0655ce2 to 3bd14c4 Compare November 1, 2018 12:56
@dmigwi
Copy link
Member Author

dmigwi commented Nov 1, 2018

fetchRowLinkURL template function simply helps to hierarchically link the list view pages i.e. /years -> /months -> /weeks -> /days -> /blocks
This simply implies that

  • every row in "/years" page has a link to "/months" page
  • every row in "/months" page has a link to "/weeks" page
  • every row in "/weeks" page has a link to "/days" page
  • every row in "/days" page has a link to "/blocks" page

chappjc
chappjc approved these changes Nov 1, 2018
View reviewed changes
Copy link
Member

@chappjc chappjc left a comment

Choose a reason for hiding this comment

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

The docs you added for fetchRowLinkURL clarified it perfectly. Thanks.

@dmigwi
Copy link
Member Author

dmigwi commented Nov 1, 2018 •
edited
Loading

Adds /years, /months, /weeks and /days pages that are hierarchically linked to each other i.e /years -> /months -> /weeks -> /days -> blocks.

This implies every row in "/years" page has a link to "/months" page, every row in "/months" page has a link to "/weeks" page, every row in "/weeks" page has a link to "/days" page and every row in "/days" page has a link to "/blocks" page.

Reduces the font-size of the drop down menu and rearranges the navigation buttons so that the right most buttons returns the history data.

☝️

@dmigwi dmigwi force-pushed the timebased-blocks-listing branch 3 times, most recently from 27d3297 to 7dfc563 Compare November 1, 2018 17:05
@dmigwi
Copy link
Member Author

dmigwi commented Nov 1, 2018

Bootstrap class table-responsive was squeezing the tables. I removed it to fix that issue. Its not even being used in most of the other tables.

Before
screenshot from 2018-11-01 20-51-07

After
screenshot from 2018-11-01 20-51-33

chappjc
chappjc reviewed Nov 1, 2018
View reviewed changes
views/windows.tmpl Outdated Show resolved Hide resolved
@dmigwi dmigwi force-pushed the timebased-blocks-listing branch 3 times, most recently from 067cbc4 to 1ffb635 Compare November 1, 2018 19:29
@chappjc
Copy link
Member

chappjc commented Nov 2, 2018

This is working very well in my testing now. At first I was seeing the blocks list pages automatically reload the homepage on some websocket event, but that seems to be sorted out after clearing cache.

The only real style change is the font size of the menu going from 20 pt to 15 pt. It's quite easy to revert that and it is not too disruptive, so I say we merge this and see how people like it during the 3.2 dev cycle.

@chappjc chappjc merged commit 39e3b0f into decred:master Nov 2, 2018
@dmigwi dmigwi deleted the timebased-blocks-listing branch February 3, 2019 19:09
mkingori pushed a commit to mkingori/dcrdata that referenced this pull request Apr 9, 2020
@dmigwi @chappjc
Timebased blocks listing (decred#779)
4c1ea52 
This adds new block listing pages differing by time window grouping.

Added pages are: /years, /months, /weeks, and /days. They hierarchically link to
each other i.e /years -> /months -> /weeks -> /days -> /blocks.

Every row in "/years" page has a link to "/months" page, every row in "/months"
page has a link to "/weeks" page, etc.

This also reduces the font-size of the drop down menu and rearranges the navigation
buttons so that the right most buttons returns the history data.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chappjc chappjc chappjc approved these changes

@gozart1 gozart1 Awaiting requested review from gozart1

Assignees
No one assigned
Labels
needs testing UI / graphics
Projects
None yet
Milestone
4.0
Development

Successfully merging this pull request may close these issues.
2 participants
@dmigwi @chappjc