Closed Bug 1385852 Opened 3 years ago Closed 3 years ago

Remove drag space above the tabs

Categories

(Thunderbird :: Theme, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 56.0

People

(Reporter: Paenglab, Assigned: Paenglab)

References

Details

Attachments

(3 files, 1 obsolete file)

Like FX has now, we could also remove the drag space above the tabs.
Attached patch dragSpace.patch (obsolete) — Splinter Review
Jörg, what do you think about this. Naturally you see only a difference when drawintitlebar is enabled.

Differently to FX I moved the drag space to the end of the tabs instead the start because TB has normally not so many tabs open and this space at the start looks not so good.

I also synchronized the space calculation code with the one from FX (browser-tabsintitlebar.js). One question, is it possible to get the "CAN_DRAW_IN_TITLEBAR" through appconstants? Then we could get rid of the precompiling of this file.

This works only on macOS and Windows because Linux can't draw into the titlebar
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8891987 - Flags: review?(jorgk)
(In reply to Richard Marti (:Paenglab) from comment #1)
> Jörg, what do you think about this. Naturally you see only a difference when
> drawintitlebar is enabled.
To see the difference, I need to switch the menu off, right?
(Note, I generally use a menu and mail.tabs.drawInTitlebar==false, so please mention differing settings in your instructions).

Where did FF introduce this, it's not in version 54. I think it looks better, since we save space and the minimise, maximise and close icons move down into the space provided for them in the tabbar.
 
> Differently to FX I moved the drag space to the end of the tabs instead the
> start because TB has normally not so many tabs open and this space at the
> start looks not so good.
This I don't understand, what's "drag space"? I don't have a current FF Nightly installed, perhaps you can attach a screen shot.

> I also synchronized the space calculation code with the one from FX
> (browser-tabsintitlebar.js). One question, is it possible to get the
> "CAN_DRAW_IN_TITLEBAR" through appconstants? Then we could get rid of the
> precompiling of this file.
Hmm, I can see |if (AppConstants.CAN_DRAW_IN_TITLEBAR) {| in CustomizeMode.jsm. Does that not work in TB? If not, maybe consult Aceman.
Attached image Drag space on FX
(In reply to Jorg K (GMT+2) from comment #2)
> (In reply to Richard Marti (:Paenglab) from comment #1)
> Where did FF introduce this, it's not in version 54. I think it looks
> better, since we save space and the minimise, maximise and close icons move
> down into the space provided for them in the tabbar.

It's in the Nightlies behind the Photon build switch since 55.

> > Differently to FX I moved the drag space to the end of the tabs instead the
> > start because TB has normally not so many tabs open and this space at the
> > start looks not so good.
> This I don't understand, what's "drag space"? I don't have a current FF
> Nightly installed, perhaps you can attach a screen shot.

Attached.

> > I also synchronized the space calculation code with the one from FX
> > (browser-tabsintitlebar.js). One question, is it possible to get the
> > "CAN_DRAW_IN_TITLEBAR" through appconstants? Then we could get rid of the
> > precompiling of this file.
> Hmm, I can see |if (AppConstants.CAN_DRAW_IN_TITLEBAR) {| in
> CustomizeMode.jsm. Does that not work in TB? If not, maybe consult Aceman.

Ah great, then I try a new patch.
> Attached.
Make it smaller so can see better what's happening :-(
But I get the idea. There is some funny space to the left of the left-most tab, right? Ugly, no?
(In reply to Jorg K (GMT+2) from comment #4)
> > Attached.
> Make it smaller so can see better what's happening :-(
> But I get the idea. There is some funny space to the left of the left-most
> tab, right? Ugly, no?

Yes, that's why I moved it to the right of the tabs. It's only remarkable when there are enough tabs to fill the tabbar.
Attached patch dragSpace.patchSplinter Review
The same patch with AppConstants instead of #ifdef.
Attachment #8891987 - Attachment is obsolete: true
Attachment #8891987 - Flags: review?(jorgk)
Attachment #8892041 - Flags: review?(jorgk)
Comment on attachment 8892041 [details] [diff] [review]
dragSpace.patch

Thanks this works for me on Windows 10. Thanks for the patch without the white-space changes.

I have checked that you've synchronised code with M-C code from
mozilla-central/browser/base/content/browser-tabsintitlebar.js
Attachment #8892041 - Flags: review?(jorgk) → review+
If you want we can wait and land it in TB 57.
Keywords: checkin-needed
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/045fa1d535b5
Remove drag space above the tabs. r=jorgk
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
(In reply to Richard Marti (:Paenglab) from comment #9)
> If you want we can wait and land it in TB 57.
Why wait?

I'll even add this to the release notes for TB 56 this time:
Various improvements in the Thunderbird themes on all platforms, especially for Windows 10.
Target Milestone: --- → Thunderbird 56.0
Depends on: 1389883
You need to log in before you can comment on or make changes to this bug.