Code review comment for lp:~martijn/udstream/gdata

Revision history for this message
Paul Hummer (rockstar) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Overall, this branch looks good. There is a slight bug in the
implementation of showing the thumbnail that should probably be fixed
before this lands (detailed below). Other than that, I'm happy to merge
this for you.

 review needs_fixing

Paul

> === modified file 'lifestream.html'
> --- lifestream.html 2008-12-12 00:16:45 +0000
> +++ lifestream.html 2008-12-27 19:16:42 +0000
> @@ -168,6 +172,19 @@ ls = {
> timetd.className = "time";
> tr.appendChild(timetd);
> td.className = "data";
> + if(thumbnail) {
> + td.img = document.createElement('img');
> + td.img.src = thumbnail ;
> +
> + $(td).hover(function(){
> + this.img.style.position = "absolute";
> + this.img.style.top = (this.offsetTop + this.offsetHeight) + "px";
> + this.img.style.left = this.offsetLeft + "px";
> + document.body.appendChild(this.img);
> + }, function(){
> + document.body.removeChild(this.img);
> + });
> + }
> tr.appendChild(td);
> var srctd = document.createElement("td");
> srctd.appendChild(document.createTextNode(src));
>

There's an issue here with the thumbnail popup. If the image pops up
under your mouse, it flickers on and off because of the mouseOut event.
I'm not sure the best way to fix this, but you'll probably need to do
some checking on the mouse position, and see if they overlap, and move
the document.removeChild() to the mouseOut event of image if it's
visible under the mouse. Complicated, I know.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAklWh2sACgkQHE2KxYYv8I+k6ACgmqkSqylWW45wuGC1kbvoupuC
xTkAn06099c2nOof5Mb2CfoVwPYK2mOa
=VSPM
-----END PGP SIGNATURE-----

review: Needs Fixing

« Back to merge proposal