Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature request] Multiline tabs should expand slightly to fill horizontal space #160

Open
redyoshi49q opened this issue Oct 13, 2020 · 5 comments

Comments

@redyoshi49q
Copy link

redyoshi49q commented Oct 13, 2020

As the extension is currently implemented, when using a multi-line tab bar, tab widths function as follows:

  • If the tabs can all fit on one line at their max width value for tabs, then they are rendered on a single line at that width, typically leaving some empty space on the right.
  • If the tabs cannot fit on one line at their max width value, but /can/ fit in one line at their min width value, then they are rendered on a single line at the width that would cause them to split the horizontal width of the tab bar equally.
  • If the tabs cannot fit on one line at their max width or min width values, then they are rendered on multiple lines at their min width value; this generally leaves empty space on the right, especially when the browser's width is slightly less than a multiple of the minimum tab width.

In the third case, I think it would look more aesthetically pleasing if the tabs were instead rendered at the same width as they would be for the largest number of tabs that qualifies for the second case (rather than the minimum width for tabs). A formula such as min-tab-width + floor( (tab-bar-width % min-width) / floor(tab-bar-width / min-tab-width) ) should work for that case, I believe (this tab width should be set when tab-count * min-tab-width exceeds tab-bar-width).

@PikachuEXE
Copy link
Member

I will respond on weekend since I am busy at work

@PikachuEXE
Copy link
Member

I think most logic about the width is in
https://github.com/tabkit/tabkit2/blob/master/src/content/tabkit.js#L6413
And I have never looked at it closely
See if you can understand any part of it and figure out what's the change required

@redyoshi49q
Copy link
Author

TL;DR: Setting https://github.com/tabkit/tabkit2/blob/master/src/content/tabkit.js#L6471 to tk.setTabMinWidth(Math.floor(availWidth / tabsPerRow)); should address the issue (line 6472 appears to be a comment containing something like this, but doesn't do rounding). It may also be appropriate to create a copy of lines 6432 and 6433 just before 6463, since the if (rows > maxRows) { block adjusts availWidth; this addresses a corner case where the loss of effective width in the tab bar caused by the presence of a scroll bar causes the number of tabs in each row to change.

I wasn't able to package the repository code into an .xpi file that functions as expected. Any attempts at installing an .xpi file that I've generated from source (by creating a zip file containing the contents of the src directory with the .xpi extension) causes Pale Moon's tab bar to render a single line of tabs that are uncolored, which suggests that part or all of the extension is not correctly loading; subsequently installing the .xpi file that's posted as the most recent release on Github restores the extension's expected functionality. It's entirely plausible that I'm overlooking something in my attempts at packaging, but I'm not entirely sure what I might be missing. As a sanity check, does an .xpi file built from a freshly checked out version of the source (either on the main branch or the v0.14.0 tag) function in the browser as expected on your end?

I was, however, able to edit the contents of an .xpi file for version 0.14.0 directly in order to test out the changes I've described in the first paragraph. It works marginally adequately, and will likely suffice for my own purposes for now. A cursory overview suggests that other sections of the code might assume that tabs are at their minimum width, so while I haven't yet observed any obvious bugs caused by the change, I can't confidently say that none exist. Also, a better solution would be to distribute the remaining handful of pixels that don't divide evenly among the tabs in each row (so the tabs would have widths that differ by at most 1 pixel from each other in order to always take up all of the available space)

Thanks for pointing out the line number of the relevant code; it made browsing the code for the relevant line much easier.

@PikachuEXE
Copy link
Member

To build an xpi file you need to

  • yarn install
  • npx gulp build_xpi_official_release

Tasks are defined in https://github.com/tabkit/tabkit2/blob/master/gulpfile.js (see last few lines with export.task_name = xxx)

@PikachuEXE
Copy link
Member

I have recently added a document about extension development

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants