Michael Yoshitaka Erlewine [:mitcho] Assignee
	Description • 14 years ago

Right now I have "first run" set to go off every time you open a new  window and you don't have any groups. We need a better rule.  Store a "we've shown firstrun" flag in Prefs; if it's not there, assume we haven't yet.

	Michael Yoshitaka Erlewine [:mitcho] Assignee
	Comment 2 • 14 years ago

Comment on attachment 471648 [details] [diff] [review]
Proposed patch

Canceling review. We will commit this to tabcandy-central and get it reviewed and landed as part of bug 578512.

	Michael Yoshitaka Erlewine [:mitcho] Assignee
	Comment 3 • 14 years ago

http://hg.mozilla.org/users/edward.lee_engineering.uiuc.edu/tabcandy-central/rev/9eb8d4396885

Committed to tabcandy-central.

	Michael Yoshitaka Erlewine [:mitcho] Assignee
	Comment 4 • 14 years ago

Comment on attachment 471648 [details] [diff] [review]
Proposed patch

We are no longer doing a t-c to m-c merge. Requesting feedback so we can review it.

	Ian Gilman [:iangilman]
	Comment 5 • 14 years ago

Comment on attachment 471648 [details] [diff] [review]
Proposed patch

looks great.

	Michael Yoshitaka Erlewine [:mitcho] Assignee
	Comment 6 • 14 years ago

I'm going to write a test for this before landing.

	Ian Gilman [:iangilman]
	Comment 7 • 14 years ago

Comment on attachment 471648 [details] [diff] [review]
Proposed patch

Random thought: what if someone sets the pref to false manually, after they already have a number of groups? As written, we will create a new group and pull all of their tabs into it, but leave the other groups around (as long as they have titles). 

I wouldn't say this scenario warrants a lot of effort, but maybe we could do something simple to address it.

	Michael Yoshitaka Erlewine [:mitcho] Assignee
	Comment 8 • 14 years ago

Patch for checkin, with a test... BUT the try tree is closed so I can't send it for a sanity test. Will push to try whenever I can.

	Raymond Lee [:raymondlee]
	Comment 9 • 14 years ago

We need to ensure that we create a new group and put all orphan tab items in if no group data is available even after the first run.

Pushed to try -> 35bb5ccd05d6

	Michael Yoshitaka Erlewine [:mitcho] Assignee
	Comment 10 • 14 years ago

Thanks Raymond. I didn't like how we split up the grouping code and the first-run welcome infoitem code, so I pulled it all out into a utility function, UI.reset(), which perhaps some of our tests can benefit from as well.

Also, in the unlikely scenario it was firstTime but there's group info, it would display the infoitem but not do the grouping, so I fixed that.

	Michael Yoshitaka Erlewine [:mitcho] Assignee
	Comment 11 • 14 years ago

Pushed to try: dca4fb9b939b

	Ian Gilman [:iangilman]
	Comment 12 • 14 years ago

Comment on attachment 476985 [details] [diff] [review]
Patch v3

Looks good, but let's destroy any existing groups in reset. Also, we can kill _reset to avoid confusion; it's out of date now anyway.

	Michael Yoshitaka Erlewine [:mitcho] Assignee
	Comment 13 • 14 years ago

Re-requesting review as the patch has changed sufficiently from the previously r+'ed patch.

	Michael Yoshitaka Erlewine [:mitcho] Assignee
	Comment 14 • 14 years ago

Patch v3, test-interaction-wise exactly the same, passed try. Patch v4 also passes locally (as does patch v3). Just waiting on review, then.

	Justin Dolske [:Dolske]
	Comment 15 • 14 years ago

Pushed http://hg.mozilla.org/mozilla-central/rev/c9bf3375b202

	Michael Yoshitaka Erlewine [:mitcho] Assignee
	Comment 16 • 14 years ago

Thanks Dolske! :D

	Tracy Walker [:tracy]
	Comment 17 • 14 years ago

verified with recent nightly trunk builds