Streamline tab prefs

RESOLVED FIXED in Camino1.5

Status

Camino Graveyard
Preferences
--
enhancement
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Stuart Morgan, Assigned: froodian (Ian Leue))

Tracking

({fixed1.8.1})

Trunk
Camino1.5
PowerPC
Mac OS X
fixed1.8.1

Details

Attachments

(5 attachments, 5 obsolete attachments)

65.29 KB, image/png
Details
41.37 KB, image/png
Details
5.18 KB, patch
Stuart Morgan
: review+
Mike Pinkerton (not reading bugmail)
: superreview+
Details | Diff | Splinter Review
6.86 KB, application/zip
froodian (Ian Leue)
: review+
Mike Pinkerton (not reading bugmail)
: superreview+
Details
47.62 KB, image/png
Details
(Reporter)

Description

11 years ago
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.
(Reporter)

Comment 1

11 years ago
Created attachment 237893 [details]
current pane

Screenshot of current pref pane, for baselining.
(Reporter)

Comment 2

11 years ago
Created attachment 237894 [details]
proposal

Mockup of a refactored version (exact wording subject to refinement, of course)
Dupe of bug 346128?
(Reporter)

Comment 4

11 years ago
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
(Reporter)

Comment 6

11 years ago
(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.

Comment 7

11 years ago
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 #7)
> Will this fix the last half of bug 346130 comment 19?

Yes.

Comment 9

11 years ago
(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
(Assignee)

Comment 10

11 years ago
A new nib incorporating the proposed changes is in bug 340412.
Depends on: 340412
(Assignee)

Updated

11 years ago
Assignee: nobody → stridey
(Assignee)

Comment 11

11 years ago
Created attachment 239086 [details] [diff] [review]
Patch
Attachment #239086 - Flags: review?(stuart.morgan)
(Assignee)

Comment 12

11 years ago
Created attachment 239087 [details]
New Tabs.nib
Attachment #239087 - Flags: review?(stuart.morgan)
(Assignee)

Comment 13

11 years ago
Created attachment 239088 [details]
Screenshot of new Tabs.nib
(Assignee)

Updated

11 years ago
Status: NEW → ASSIGNED
No longer depends on: 340412
(Reporter)

Comment 14

11 years ago
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-
(Reporter)

Comment 15

11 years ago
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-
(Assignee)

Comment 16

11 years ago
Created attachment 239352 [details] [diff] [review]
Patch
Attachment #239086 - Attachment is obsolete: true
Attachment #239352 - Flags: review?(stuart.morgan)
(Assignee)

Comment 17

11 years ago
Created attachment 239353 [details]
New Tabs.nib
Attachment #239087 - Attachment is obsolete: true
Attachment #239353 - Flags: review?(stuart.morgan)
(Assignee)

Comment 18

11 years ago
Created attachment 239354 [details]
Screenshot of new Tabs.nib
Attachment #239088 - Attachment is obsolete: true
(Assignee)

Updated

11 years ago
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.
(Reporter)

Comment 20

11 years ago
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+
(Assignee)

Comment 21

11 years ago
Created attachment 239503 [details]
New Tabs.nib
Attachment #239353 - Attachment is obsolete: true
Attachment #239503 - Flags: review?(stuart.morgan)
Attachment #239353 - Flags: review?(stuart.morgan)
(Assignee)

Comment 22

11 years ago
Created attachment 239504 [details]
Screenshot of new Tabs.nib
Attachment #239354 - Attachment is obsolete: true
(Reporter)

Comment 23

11 years ago
Comment on attachment 239353 [details]
New Tabs.nib

r=me with pink's change
Attachment #239353 - Flags: review+
(Assignee)

Comment 24

11 years ago
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+
(Assignee)

Comment 27

11 years ago
Checked in on 1.8branch and trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.