Quicksaver / Puzzle-Toolbars

A Firefox that adds many toolbar choices to fully customize the browser window.
https://addons.mozilla.org/firefox/addon/puzzle-toolbars/
Mozilla Public License 2.0
19 stars 4 forks source link

Some buttons appear too wide in the location bar #34

Closed Keith94 closed 10 years ago

Keith94 commented 10 years ago

The buttons from: https://addons.mozilla.org/addon/59961 https://addons.mozilla.org/addon/478363 https://addons.mozilla.org/addon/509562

Their images are stretched: snag 2014 05 12 16 11 40

I'm guessing these buttons all have something in common that's causing the issue, and you'll know what that "something" is...

Quicksaver commented 10 years ago

It's probably something in common yeah, I'll check them out (this weekend). Does this only happen in the location bar?

Keith94 commented 10 years ago

Does this only happen in the location bar?

Yes, I was going to mention that before.

Quicksaver commented 10 years ago

Alright, I'm guessing this won't be too hard to fix. Thanks for letting me know!

Keith94 commented 10 years ago

No worries.

Keith94 commented 10 years ago

Hmm, are the dimensions of TU icons still wrong?

button_tuOptions = 20x18 in location bar, but in other placements it's 16x16. closetab-button = 10x10 in location bar, other placements it's only 8x8.

Quicksaver commented 10 years ago

button_tuOptions = 20x18 in location bar, but in other placements it's 16x16.

That's because it forcefully sets itself to 16x16 in other places. I wasn't sure how to proceed with it, I think its size hasn't been updated to fit Australis properly, which expects button icons to be 18px and not 16px anymore. And since the original icon is 32x27, I gave it a similar ratio so it's not distorted. But maybe it should be TU's developer making these calls and not me, I'll just make it so it appears like in the other add-on bar locations and doesn't stretch out like before.

closetab-button = 10x10 in location bar, other placements it's only 8x8.

That one was my bad. :)

Keith94 commented 10 years ago

Ahh, so how come Stylish's icon is 18px in the corner/bottom add-on bar, when in the navigation bar and other places it's 16px? I didn't see any 18px icon files in the source for some add-ons that I checked.

Quicksaver commented 10 years ago

That's because they (mozilla) allowed 16px icons to still work, to facilitate the migration from pre-australis, by adding a 1px padding (or margin or border, don't remember exactly but I think it's padding) to the icon, so that the same icon can still be used in both versions.

Obviously, this can lead to some "problems" if the add-ons enforce an icon size on their own, over the firefox defaults and then another add-on like mine tries to change that. For example, TPP only sets its max-height:18px, to ensure it's not above that, but Stylish uses a 16px icon when it's a child of the nav-bar, which technically is true. Stylish does this because it's a valid thing to do, since when placed in the nav-bar, the icon will still be rendered correctly without increasing the size of its button.

So, TPP doesn't stretch the icon to 18px, it only ensures it doesn't go beyond it. Since Stylish actually uses a 16px icon (and the extra padding rules for compatibility don't actually apply to the add-on bar in this case), it appears smaller.

Keith94 commented 10 years ago

Sorry if I'm misunderstanding, but the Stylish icon looks worse at the bottom/corner (18px), than in the location bar (16px): stylish

Adblock Plus too, if this is a better comparison: abp

Is that a real issue or no?

Quicksaver commented 10 years ago

In the corner/bottom, the stylish button uses a 24px icon file, while in the location bar it uses a 16px icon file; this comes from stylish's own stylesheets.

In the corner/bottom, this 24px icon is reduced to 18px, so there's some detail loss for compressing it, while in the location bar the 16px isn't increased to 18px, it stays at 16px with a little padding border around it to compensate (even if there's no actual padding added, which seems the case from what I see, the icon is still centered, so it's like there was some padding anyway), so there's no detail loss from expanding.

Ideally, it should use the same icon size in both places (18px) of course, but I don't think that's a change I should be doing in TPP and it should come from the add-on itself. (There must be dozens of add-ons in similar situations, if not hundreds. There's no way I can "keep a list".)

I didn't check ABP's button, but I assume it's something similar.

Keith94 commented 10 years ago

But I'm not sure why you would deem the 24px->18px resized icons to be ideal, when they end up looking worse than the 16px files (with some padding rule or such).

In some other toolbars I see the iconsize=small attribute used, is there some reason you don't use that in your australisBar.xul file as well?

Quicksaver commented 10 years ago

But I'm not sure why you would deem the 24px->18px resized icons to be ideal, when they end up looking worse than the 16px files (with some padding rule or such).

I could make it use the 16px icon, but this is what I meant that it should be stylish doing and not TPP. And I don't deem anything in regards to stylish, I actually don't do anything to it specifically. It is stylish itself that determines to use the 16px icon, not TPP. Which is why it should be stylish to correct this (circling back to the whole "can't keep a list of all the add-ons that do this" thing).

In some other toolbars I see the iconsize=small attribute used, is there some reason you don't use that in your australisBar.xul file as well?

This attribute is no longer used in australis, as there is no longer a size distinction. I guess that specific value was left in the toolbars (I hadn't even realized this until now) to make sure any stylesheets that might require it will still work. I think I'll add that attribute as well as you suggested, it might solve these issues, although it might also not.

Quicksaver commented 10 years ago

In some other toolbars I see the iconsize=small attribute used, is there some reason you don't use that in your australisBar.xul file as well?

This attribute is no longer used in australis, as there is no longer a size distinction. I guess that specific value was left in the toolbars (I hadn't even realized this until now) to make sure any stylesheets that might require it will still work. I think I'll add that attribute as well as you suggested, it might solve these issues, although it might also not.

So after a quick test there's something I don't like about this. The stylish button becomes visibly smaller than the others in the corner/bottom, and that looks just a little too weird (it feels uneven to mouse over them). While in the location bar (which is precisely because the nav-bar has this attribute that stylish uses the 16px icon) this size difference isn't there (although I actually have no idea why, I'll investigate) because the actual button stays the same size.

So basically, as it is now, even though the stylish icon is different depending on the placement of the add-on bar, the actual (clickable) button stays uniform with the default buttons, whereas using that attribute it would not. Also, I think it's an exaggeration when you say "it looks worse", yes it's true that it loses a little detail, but at that size that's hardly noticeable.

Worse part is, I'm certain that to fix this, I would have to target the stylish icon separately, and we're entering the "can't keep a list" issue again.

Quicksaver commented 10 years ago

So I made some changes and I think this can make everyone happy, myself included.

Keith94 commented 10 years ago

Great solution!

The zoom/edit controls buttons could use some work, and the separators in some cases look off: snag 2014 05 15 04 08 01

Quicksaver commented 10 years ago

Actually it's not such a great solution as I expected, as I've tried it in my main profile and it introduces other problems (the top alignment kills a lot of buttons, not just the separators and zooms). I'm working on another solution that will hopefully be better. ;)

Quicksaver commented 10 years ago

I should have done this from the start!

This works perfectly for me, please let me know how it does for you.

Keith94 commented 10 years ago

Yeah it's much better now.

Some additional things you could fix:

The zoom reset button label is not displayed. The search bar could use improvement. The toggle button overlaps the rightmost button a tiny bit.

Quicksaver commented 10 years ago

The zoom reset button label is not displayed.

Yep you're right. This will have to work on an "exception-to-the-general-rule" basis. TPP hides the button texts automatically by default, but some should still have them of course like this one. If you find any other button that should have its text visible but doesn't (or the opposite) please let me know.

The search bar could use improvement.

Tweaked it a little, it should be presentable now. Also, if you enable autohide and focus the search bar, the add-on bar will still autohide if you move the mouse away. To fix this I need a couple of updates that are included in The Fox, Only Better, but that would remove compatibility with older firefox versions (technically I could adapt the code but why bother if I can copy paste everything later; yes I'm uber lazy).

I only plan on doing this merge around firefox 30 (because of OmniSidebar which is using the exact same files as TPP, reasons to do with changes to the Social Sidebar introduced in FF30, so I'll remove all backwards compatibility then), so around that time I will be able to fix this issue with the searchbar focused easily.

The toggle button overlaps the rightmost button a tiny bit.

Fixed.

Keith94 commented 10 years ago

Nice~

There are some small inconsistencies in the location bar compared to other placements.

And how about specifying a min/max width for the search bar? At the bottom it's very wide and other places it's quite narrow.

Quicksaver commented 10 years ago

Fixed on all counts (I think), except for the search bar in the add-on bar on the bottom. That's a feature of the search bar, it's supposed to stretch to fit all the available space, it only doesn't stretch more in the nav-bar because of the location bar which does the same and has a higher flex number. If you place the search bar in the bookmarks toolbar, it will do the same.

Keith94 commented 10 years ago

A few fixes are incomplete:

#zoom-controls,
#edit-controls {
    margin: 0 -1px !important;
}

And I noticed that the add-on bar width doesn't "update" properly when I make any CSS changes to the buttons, but adding this fixed it for me:

#thePuzzlePiece-urlbar-addonbar-container {
    width: auto;
}
Quicksaver commented 10 years ago

And I noticed that the add-on bar width doesn't "update" properly when I make any CSS changes to the buttons, but adding this fixed it for me:

That's because CSS changes aren't registered on any level in javascript code, so TPP can't follow them blindly. It only updates its width when an "action" takes place. By using that piece of code, you lost the transition effects when opening/closing/hovering the add-on bar. ;)

Keith94 commented 10 years ago

Oops, you're right. :P Nevermind.

Quicksaver commented 10 years ago

Fixed on all counts (except for not updating on CSS changes thing because of the above).

Your eyes are amazing by the way! I never would have spotted so many things in such a little time...

Keith94 commented 10 years ago

Oh thanks. :)

Looks like there's nothing left to fix in this department.

Keith94 commented 10 years ago

If you find any other button that should have its text visible but doesn't (or the opposite) please let me know.

For add-ons like Tab Grenade and Save Text To File, their buttons are only displayed with text when in customize mode, so if you move them to the add-on bar their text is hidden so you can't identify them.