65.29 KB, image/png
41.37 KB, image/png
5.18 KB, patch
|Details | Diff | Splinter Review|
6.86 KB, application/zip
47.62 KB, image/png
Looking at the tab prefs, I realized that there are essentially three whole one-element groups all having to do with the same thing: do you want tabs or windows for <foo>. Not only that, but two use radio buttons, and one uses checkboxes. One might call it "awkward and crowded". I think they could all be distilled down to one category about using tabs, leading to a much more streamlined pref pane.
Dupe of bug 346128?
Not a dup; I removed 0 configurability. All the power, half the calories.
Yeah, this is very good. Fresh set of eyes :) I think the text needs a few tweaks (and some hint text again), but the basic idea is fine. a=me ;) Specifically, * keep "Command" with the symbol * the Tab Bar text is seems odd, but no suggestion atm * Hint text (somehow?) along the lines of "'aka' single window mode" and "This applies to links you click on in Mail, iChat, Address Book, etc." for the latter 2 in the first group (note the new "you" in the second hint) * Hint text along the lines of "otherwise, these open in new windows" for the first group? This even opens the way for a "use tabs" checkbox that automatically checks all of the first group when checked, to save beng two clicks ;)
Target Milestone: --- → Camino1.1
(In reply to comment #5) > * keep "Command" with the symbol Why? Are there users who know the name and not the symbol? > * the Tab Bar text is seems odd, but no suggestion atm It's odd because the part to the right of the ':' doesn't continue the part on the left, and is now alone in that respect. I didn't like it either, but I didn't spend a lot of time messing with it. > * Hint text (somehow?) along the lines of "'aka' single window mode" Why? That's a moz-geek term that doesn't mean anything to any but a vanishingly small number of users. > "This applies to links you click on in Mail, iChat, Address Book, etc." Users don't know what "other" means? That seems unlikely to me.
Will this fix the last half of bug 346130 comment 19? > additionally, how are we treating the inconsistency between the hidden pref > [reuse window when opening links from other applications] > and radio grid? unlike checkboxes, which can now show a "mixed" state for > user-modified prefs, this implementation has no way of showing what's > actually in your prefs file and no way of finding out if the current state, > as displayed, is actually what's in your prefs files.
(In reply to comment #6) > (In reply to comment #5) > > * keep "Command" with the symbol > > Why? Are there users who know the name and not the symbol? There's definitely a class of users who don't associate the two (the name and the symbol) as being related. Since there's plenty of horizontal space, I think it'd be nice to have the word "Command" in there too. cl
Status: NEW → ASSIGNED
No longer depends on: 340412
Comment on attachment 239087 [details] New Tabs.nib r-; per IRC: Tab Bar: Always stays visible to read better, and drop "Command" from the text (and add a hyphen).
Attachment #239087 - Flags: review?(stuart.morgan) → review-
Comment on attachment 239086 [details] [diff] [review] Patch >+ IBOutlet NSButton* mCheckboxOpenTabsForCommand; >+ IBOutlet NSButton* mCheckboxOpenTabsForExternalLinks; >+ IBOutlet NSButton* mSingleWindowMode; >+ >+ IBOutlet NSButton* mCheckboxLoadTabsInBackground; >+ IBOutlet NSButton* mTabBarVisiblity; Axe all the extra whitespace in here >+ [mCheckboxOpenTabsForCommand setState:[self getBooleanPref:"browser.tabs.opentabfor.middleclick" withSuccess:&gotPref] ? NSOnState : NSOffState]; ... For legibility, parenthesize all non-unit arguments (e.g., |setState:([...] ? NSOnState : NSOffState)|). There's a bunch of these. >+ int newState = [sender state] == NSOnState ? nsIBrowserDOMWindow::OPEN_NEWTAB : nsIBrowserDOMWindow::OPEN_DEFAULTWINDOW; Similarly, |([sender state] == NSOnState) ?|
Attachment #239086 - Flags: review?(stuart.morgan) → review-
Attachment #239353 - Attachment description: New Tab.nib → New Tabs.nib
i think the first one reads better as: Open tabs instead of windows for: Links opened with cmd-click The way it's written in the new nib doesn't really make sense gramatically.
Comment on attachment 239352 [details] [diff] [review] Patch r=me
Comment on attachment 239353 [details] New Tabs.nib r=me with pink's change
Attachment #239353 - Flags: review+
Comment on attachment 239503 [details] New Tabs.nib Wheee, cross postination. This is r=smorgan.
Comment on attachment 239352 [details] [diff] [review] Patch sr=pink
Attachment #239352 - Flags: superreview?(mikepinkerton) → superreview+
Comment on attachment 239503 [details] New Tabs.nib sr=pink
Attachment #239503 - Flags: superreview?(mikepinkerton) → superreview+
Checked in on 1.8branch and trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.