Closed
Bug 352265
Opened 18 years ago
Closed 18 years ago
Streamline tab prefs
Categories
(Camino Graveyard :: Preferences, enhancement)
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)
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
stuart.morgan+bugzilla
:
review+
mikepinkerton
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
application/zip
|
froodian
:
review+
mikepinkerton
:
superreview+
|
Details |
(deleted),
image/png
|
Details |
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•18 years ago
|
||
Screenshot of current pref pane, for baselining.
Reporter | ||
Comment 2•18 years ago
|
||
Mockup of a refactored version (exact wording subject to refinement, of course)
Comment 3•18 years ago
|
||
Dupe of bug 346128?
Reporter | ||
Comment 4•18 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•18 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.
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•18 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•18 years ago
|
||
A new nib incorporating the proposed changes is in bug 340412.
Depends on: 340412
Assignee | ||
Updated•18 years ago
|
Assignee: nobody → stridey
Assignee | ||
Comment 11•18 years ago
|
||
Attachment #239086 -
Flags: review?(stuart.morgan)
Assignee | ||
Comment 12•18 years ago
|
||
Attachment #239087 -
Flags: review?(stuart.morgan)
Assignee | ||
Comment 13•18 years ago
|
||
Reporter | ||
Comment 14•18 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•18 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•18 years ago
|
||
Attachment #239086 -
Attachment is obsolete: true
Attachment #239352 -
Flags: review?(stuart.morgan)
Assignee | ||
Comment 17•18 years ago
|
||
Attachment #239087 -
Attachment is obsolete: true
Attachment #239353 -
Flags: review?(stuart.morgan)
Assignee | ||
Comment 18•18 years ago
|
||
Attachment #239088 -
Attachment is obsolete: true
Assignee | ||
Updated•18 years ago
|
Attachment #239353 -
Attachment description: New Tab.nib → New Tabs.nib
Comment 19•18 years ago
|
||
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•18 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•18 years ago
|
||
Attachment #239353 -
Attachment is obsolete: true
Attachment #239503 -
Flags: review?(stuart.morgan)
Attachment #239353 -
Flags: review?(stuart.morgan)
Assignee | ||
Comment 22•18 years ago
|
||
Attachment #239354 -
Attachment is obsolete: true
Reporter | ||
Comment 23•18 years ago
|
||
Comment on attachment 239353 [details]
New Tabs.nib
r=me with pink's change
Attachment #239353 -
Flags: review+
Assignee | ||
Comment 24•18 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 25•18 years ago
|
||
Comment on attachment 239352 [details] [diff] [review]
Patch
sr=pink
Attachment #239352 -
Flags: superreview?(mikepinkerton) → superreview+
Comment 26•18 years ago
|
||
Comment on attachment 239503 [details]
New Tabs.nib
sr=pink
Attachment #239503 -
Flags: superreview?(mikepinkerton) → superreview+
Assignee | ||
Comment 27•18 years ago
|
||
Checked in on 1.8branch and trunk
You need to log in
before you can comment on or make changes to this bug.
Description
•