Closed Bug 352265 Opened 14 years ago Closed 14 years ago

Streamline tab prefs

Categories

(Camino Graveyard :: Preferences, enhancement)

PowerPC
macOS
enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Camino1.5

People

(Reporter: stuart.morgan+bugzilla, Assigned: froodian)

References

Details

(Keywords: fixed1.8.1)

Attachments

(5 files, 5 obsolete files)

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.
Attached image current pane
Screenshot of current pref pane, for baselining.
Attached image proposal
Mockup of a refactored version (exact wording subject to refinement, of course)
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 ;)
Blocks: 325880
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
A new nib incorporating the proposed changes is in bug 340412.
Depends on: 340412
Assignee: nobody → stridey
Attached patch Patch (obsolete) — Splinter Review
Attachment #239086 - Flags: review?(stuart.morgan)
Attached file New Tabs.nib (obsolete) —
Attachment #239087 - Flags: review?(stuart.morgan)
Attached image Screenshot of new Tabs.nib (obsolete) —
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-
Attached patch PatchSplinter Review
Attachment #239086 - Attachment is obsolete: true
Attachment #239352 - Flags: review?(stuart.morgan)
Attached file New Tabs.nib (obsolete) —
Attachment #239087 - Attachment is obsolete: true
Attachment #239353 - Flags: review?(stuart.morgan)
Attached image Screenshot of new Tabs.nib (obsolete) —
Attachment #239088 - Attachment is obsolete: true
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
Attachment #239352 - Flags: superreview?(mikepinkerton)
Attachment #239352 - Flags: review?(stuart.morgan)
Attachment #239352 - Flags: review+
Attached file New Tabs.nib
Attachment #239353 - Attachment is obsolete: true
Attachment #239503 - Flags: review?(stuart.morgan)
Attachment #239353 - Flags: review?(stuart.morgan)
Attachment #239354 - Attachment is obsolete: true
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.
Attachment #239503 - Flags: superreview?(mikepinkerton)
Attachment #239503 - Flags: review?(stuart.morgan)
Attachment #239503 - Flags: review+
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
Closed: 14 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.