Scroll BubbleCalendar year chart horizontally and tooltips vertically #332
Conversation
…wheeled horizontal scrollbar.
…lled horizontally when user selects a year.
There was a problem hiding this comment.
Hi @MohammedElsayyed, I looked at this in use at http://web.archive.bibalex.org/web/20050915000000*/http://google.com, and it seems to be working well and an improvement on the way the tooltips in BubbleCalendar had been functioning previously.
I think the PR mostly looks good--just had comments regarding minor code cleanup. I am assigning @runderwood as a reviewer on this too because he has greater knowledge regarding JavaScript than me.
Thanks!
| @@ -0,0 +1,9 @@ | |||
| var x = "Mohamed"; | |||
| console.log( x ); | |||
There was a problem hiding this comment.
Seems like we shouldn't need the first two lines here.
| @@ -239,7 +244,7 @@ $().ready(function(){ | |||
|
|
|||
| <div class="clearfix"></div> | |||
|
|
|||
| <div id="wbChart" onmouseout="showTrackers('none'); setActiveYear(startYear);"> | |||
| <div id="wbChart" onmouseout="showTrackers('none'); setActiveYear(startYear);" style=" width: 963px; height: 118px;" > | |||
|
|
|||
There was a problem hiding this comment.
superfluous space in style=" width:
| int monthWidth = 5; | ||
| int startYear = Timestamp.getStartYear(); | ||
|
|
||
| for (int year = startYear; year <= Calendar.getInstance().get(Calendar.YEAR); year++) | ||
| imgWidth += yearWidth; | ||
| imgWidth = yearWidth * ( Calendar.getInstance().get(Calendar.YEAR) - startYear + 1 ); | ||
|
|
There was a problem hiding this comment.
Strictly a style comment, but in order to maintain consistency with the pre-existing code in this file, it makes sense to me to remove the extra whitespace after the open parentheses and before the close parentheses and in other changes made in this PR that add extra whitespace inside parentheses and curly braces.
|
@MohammedElsayyed It doesn't appear to be part of this pull request, but it looks to me (and I'm unfamiliar w/ the code) that there's no header included in BubbleCalendar.jsp. Shouldn't there be something like this before L17?
|
There was a problem hiding this comment.
The behavior is way, way better, to me. Code looks good.
As for the header issue (seems to be missing from BubbleCalendar.jsp) which doesn't originate in @MohammedElsayyed's branch, should I open an issue and fix that myself? @ldko
|
@runderwood, it is problematic that we aren't including UI-header.jsp. I will open an issue to discuss this as I think handling it properly will require more than just an include of the existing header. I also noticed that in BubbleCalendar.jsp the submit button |
|
@runderwood I agree with @ldko. Thank you for opening this issue. @ldko of course, I will update the pull request. Thanks! |
Brings back @MohammedElsayyed commits from #332.
This pull request fixes #214 and #328 .
Add a horizontal scrollbar to BubbleCalendar year chart to avoid exceeding window width in the future.
Make BubbleCalendar tooltips scrollable to avoid exceeding window height when a specific day contains too many snapshots.
I merged changes done in issue #214 (PR #317) into this PR. I expect this branch 'scrollbars' will handle all scroll-related issues.