Skip to content
This repository has been archived by the owner. It is now read-only.

[dev.icinga.com #2855] javascript refresh jumps to page top, not restoring previous scrolling position; add url param 'scroll=' #1025

Closed
icinga-migration opened this issue Jul 16, 2012 · 17 comments

Comments

Projects
None yet
1 participant
@icinga-migration
Copy link
Member

commented Jul 16, 2012

This issue has been migrated from Redmine: https://dev.icinga.com/issues/2855

Created by ralfk on 2012-07-16 07:24:00 +00:00

Assignee: mfriedrich
Status: Resolved (closed on 2012-09-18 23:53:30 +00:00)
Target Version: 1.8
Last Update: 2014-12-08 09:27:23 +00:00 (in Redmine)


In previous Icinga releases (e.g. 1.6) the page refresh did not change the vertical position of the page. That means, if a user scrolled down to the bottom of the page in order to view a certain part of the page, the page position wasn't changed after the automated refresh.

This behavior changed after Icinga 1.7 introduced a new refresh method based on JavaScript (see https://dev.icinga.org/issues/2119 and https://www.icinga.org/2012/05/15/icinga-1-7-released/). Now, after a page refresh the browser always jumps to the top of the page.

Is there a chance to implement the old behavior into the new JavaScript code?

Changesets

2012-09-11 17:27:32 +00:00 by mfriedrich 348658c

classic ui: javascript refresh jumps to page top, not restoring previous scrolling position; add url param 'scroll=' #2855 - MF

the javascript refresh introduced in previous versions did one thing
wrong - the client side does not know about the current scrolling
position, and therefore the javascript reload was jumping always to
top. being on the client, you cannot store values on reloads in sessions
or similar, but would have to use the cookie method (which is ugly).
a different approach is to add a new GET param, scroll, which gets set
and interpreted by the javascript refresh methods.

since we cannot modify dom objects when reloading the page, but only
when fully loaded, we also need to register eventhandlers in order to
restore the scroll position when we are allowed to.

since the value getters of the actual scroll position are somewhat
different (hey internet exploder 6), this will require more tests,
but the initial implementation should take care of the 3 different
object providers in the first place.

refs #2855

2012-09-11 17:36:28 +00:00 by mfriedrich 39437ca

classic ui: silence forgotten debug output from #2855

refs #2855

Relations:

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Jul 16, 2012

Updated by ralfk on 2012-07-16 07:32:48 +00:00

See e.g. http://www.redips.net/javascript/maintain-scroll-position/ or http://codesnipp.it/javascript/scroll-to-anywhere-on-the-page-in-jquery on how to keep scroll position. Maybe that helps.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Jul 16, 2012

Updated by mfriedrich on 2012-07-16 08:40:52 +00:00

  • Category set to 52
  • Status changed from New to Assigned
  • Assigned to set to ricardo
  • Target Version set to 1.8
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Sep 10, 2012

Updated by mfriedrich on 2012-09-10 18:03:41 +00:00

i can confirm that behaviour.

thing is - if we put up javascript to modify the url, we should possibly think of putting the complete cgi plus query string into it as well, not forgetting the framesource, like thruk does it. problem will be that the index.html is static and cannot determine which cgi to be called in the first place.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Sep 10, 2012

Updated by mfriedrich on 2012-09-10 18:05:18 +00:00

though, putting javascript into index.html could possibly solve that issue as well, instead of thinking somewhat dynamic language like we had php, or could go perl now.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Sep 10, 2012

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Sep 11, 2012

Updated by mfriedrich on 2012-09-11 16:22:04 +00:00

  • File added restore_scroll_after_reload.diff

ok, for the original problem - i'm not really a javascript coder, nor do i know how to keep state during reloads within javascript (is that event possible without url tricks or cookies?).

thing is, all browsers out there will know to use "ScrollTo", but the detection of the current scroll position is somewhat different.

  • window.PageYOffset - most browsers
  • document.documentElement.scrollTop - IE6
  • document.body.scrollTop - IE quirks mode

attached is a try to get the current scroll position into a global variable, and after the reload, re-reading it again, scrolling the page.

seems that the value is not kept within javascript / local cache.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Sep 11, 2012

Updated by mfriedrich on 2012-09-11 16:42:47 +00:00

ok, using the url set trick (like we used to do previously with paused or not) from http://www.redips.net/javascript/maintain-scroll-position/?param1=a&param2=b&param3=c&scroll=319 it works, on a single page.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Sep 11, 2012

Updated by mfriedrich on 2012-09-11 17:17:12 +00:00

that will possibly add a new javascript interpreted GET param onto the cgis which have page refreshing enabled.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Sep 11, 2012

Updated by mfriedrich on 2012-09-11 17:18:46 +00:00

  • Assigned to changed from ricardo to mfriedrich

it will require a dedicated eventhandler, like described in the url, as you can only modify DOM elements, when the page is fully loaded - not directly after pushing the reload itsself.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Sep 11, 2012

Updated by mfriedrich on 2012-09-11 17:24:40 +00:00

  • Tracker changed from Feature to Bug
  • Subject changed from Page refresh jumps the page to the top since Icinga 1.7 to javascript refresh jumps to page top, not restoring previous scrolling position
  • (unknown custom field) set to 1
  • (unknown custom field) set to Debian 6
  • (unknown custom field) set to Chromium 21
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Sep 11, 2012

Updated by mfriedrich on 2012-09-11 17:26:35 +00:00

  • Tracker changed from Bug to Feature
  • Subject changed from javascript refresh jumps to page top, not restoring previous scrolling position to javascript refresh jumps to page top, not restoring previous scrolling position; add url param 'scroll='

ok, more of a feature when adding something new.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Sep 11, 2012

Updated by mfriedrich on 2012-09-11 17:34:04 +00:00

  • Status changed from Assigned to 7
  • Done % changed from 0 to 80

in dev/cgis - please test!

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Sep 11, 2012

Updated by mfriedrich on 2012-09-11 17:34:09 +00:00

  • File deleted restore_scroll_after_reload.diff
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Sep 12, 2012

Updated by mfriedrich on 2012-09-12 11:22:47 +00:00

tested ok on

debian sid with

  • iceweasel 10.0.7
  • chromium 21.0.1180.89 Debian wheezy/sid (154005)
  • Opera/9.80 (X11; Linux x86_64; U; de) Presto/2.10.289 Version/12.02

as well as windows 7 x64

  • internet exploer 9.0.8112.16421
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Sep 18, 2012

Updated by ricardo on 2012-09-18 23:53:30 +00:00

  • Status changed from 7 to Resolved
  • Done % changed from 80 to 100

Tested ok as well

on OSX 10.8

  • Firefox 15.0.1
  • Chrome 21.0.1180.89 (srly?)
  • Safari 6.0

on Windows XP

  • IE 7.0

thanks Michael

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 19, 2012

Updated by ralfk on 2012-10-19 11:16:30 +00:00

Thanks for fixing it. Well done.
Just tested it on Icinga 1.8.0 under Linux with FF 16.0.1.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Dec 8, 2014

Updated by mfriedrich on 2014-12-08 09:27:23 +00:00

  • Project changed from 19 to Core, Classic UI, IDOUtils
  • Category changed from 52 to Classic UI
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.