697410 - Hide snippets container when it's empty

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Frank Yan (:fryn)]

Attachment: patch

Sometimes the snippets take a while to load, and during that time, the snippets container is empty and looks really awkward.

Let's hide the snippets container when it's empty.

Comment 1

[Marco Bonardo [:mak]]

Comment on attachment 569655 [details] [diff] [review]
patch

Review of attachment 569655 [details] [diff] [review]:
-----------------------------------------------------------------

At first glance I seemed to recall a reason it was not hidden, but actually I can't really think of any, even looking at current code.
So, provided you tested this working, it's fine to test it live.

My only doubt regards whether this may cause the session restore button to jump up and down even by a larger amount than it already does today.
I wonder if we should somehow "fix" its position, even if in a followup, maybe relative to the bottom of contentContainer?

Comment 2

[Marco Bonardo [:mak]]

(In reply to Marco Bonardo [:mak] from comment #1)
> My only doubt regards whether this may cause the session restore button to
> jump up and down even by a larger amount than it already does today.
> I wonder if we should somehow "fix" its position, even if in a followup,
> maybe relative to the bottom of contentContainer?

And actually, this may be a problem if a user tries to quickly click on the restore session button, but the snippets button appears pushing it down. he'll end up clicking on the snippet.

Comment 3

[Frank Yan (:fryn)]

(In reply to Marco Bonardo [:mak] from comment #2)
> (In reply to Marco Bonardo [:mak] from comment #1)
> > My only doubt regards whether this may cause the session restore button to
> > jump up and down even by a larger amount than it already does today.
> > I wonder if we should somehow "fix" its position, even if in a followup,
> > maybe relative to the bottom of contentContainer?
> 
> And actually, this may be a problem if a user tries to quickly click on the
> restore session button, but the snippets button appears pushing it down.
> he'll end up clicking on the snippet.

This is already a problem.
While the snippet is loading or if it fails to load:
https://people.mozilla.com/~fyan/screenshots/empty.png
After the snippet loads:
https://people.mozilla.com/~fyan/screenshots/snippet.png

The only way to fix this is to fix the space that taken up by the snippet.

Comment 4

[Frank Yan (:fryn)]

I just realized that I am using two different definitions of "fix".

A disambiguation:
The only way to resolve this problem is to maintain a constant vertical space for the snippet. Do we want to do that?

Comment 5

[Frank Yan (:fryn)]

I guess we could also animate the height of the snippet when it appears, but that seems to me like solving the wrong problem. It would be better to ensure that the snippet loads immediately, as it doesn't seem to happen reliably on any of my builds right now. (The problem doesn't seem to be platform-sensitive, since I test various versions of Firefox on machines running Snow Leopard, Lion, Windows 7, and Ubuntu 11.04.)

Fixing the initialization problem shouldn't block the landing of this little patch anyway, so I'll go ahead and land it later today.

Comment 6

[Marco Bonardo [:mak]]

(In reply to Frank Yan (:fryn) from comment #3)
> This is already a problem.

Yes, the slight difference is that now I see 2 separate click targets (that may wrongly overlap) and I target a bit lower, while after the change there is a single target, that gets literally replaced under my mouse. The probability of a mis-click seem to increase. Could maybe be avoided by using hidden instead of display:none? Is it worth it in your opinion?

Btw, currently we can't argue on the height of the snippet or use a fixed height, for example some time ago an experiment was done putting more contents in it (like the runfield demo).
I don't see a clean way to solve the mis-click issue, unless we completely remake the positioning in the page, so I'm probably lean to just follow-up on this issue, to figure out a better solution.

Comment 7

[Frank Yan (:fryn)]

(In reply to Marco Bonardo [:mak] from comment #6)
> Could maybe be avoided by using
> hidden instead of display:none?

Yeah, I thought the same.
I'll land it with `visibility: hidden;` instead of `display: none;`.

> I don't see a clean way to solve the mis-click issue, unless we completely
> remake the positioning in the page

Since the "Restore Previous Session" block seems to appear instantly reliably when it's supposed to, while the snippets block is less reliable, we could reverse the ordering of the two blocks, so no shifting will ever happen. It might make the page's visual design look wonky though.

Yeah, let's deal with this in a followup bug.

Comment 8

[Frank Yan (:fryn)]

https://hg.mozilla.org/mozilla-central/rev/7fd51fabe5d5