Skip to content
This repository has been archived by the owner on Nov 17, 2022. It is now read-only.

Make compact (many in one row) pinned tabs configurable. #177

Closed
wants to merge 1 commit into from

Conversation

dos1
Copy link
Contributor

@dos1 dos1 commented Oct 1, 2017

While pinned tabs being outside of scrolling container are awesome,
for old Tab Center-like configurations with autoshrinking sidebar
the presentation of many tabs in one row is actually an antifeature.

This PR makes it configurable, so custom userChrome.css configs like
in #103 or #118 that restore old behavior can work properly.

Also, the thumbnail reseting has been removed. This fixes a newly
introduced issue of unpinned tab not having its thumbnail updated.
While this could be fixed in other way, disabling thumbnail generation
for pinned tabs is not desired when compact pins are disabled anyway.

While pinned tabs being outside of scrolling container are awesome,
for old Tab Center-like configurations with autoshrinking sidebar
the presentation of many tabs in one row is actually an antifeature.

This PR makes it configurable, so custom userChrome.css configs like
in eoger#103 or eoger#118 that restore old behavior can work properly.

Also, the thumbnail reseting has been removed. This fixes a newly
introduced issue of unpinned tab not having its thumbnail updated.
While this could be fixed in other way, disabling thumbnail generation
for pinned tabs is not desired when compact pins are disabled anyway.
@eoger
Copy link
Owner

eoger commented Oct 5, 2017

Give me some time to think about this.
I really really don't want to have to maintain 2 implementations of the same feature.

However, I'm aware that a number of users like you have (beautiful) CSS customizations that probably rely on the pinned tabs being displayed as rows.
If I left the pinned icon in the webextension files, would you be able to reproduce the "legacy" look with CSS only? We haven't made a lot of markup changes, so I believe it might be possible.
It would be a win-win: you get to keep your pretty CSS customization while it lifts the burden of maintaining on second implementation of me. What do you think?

@dos1
Copy link
Contributor Author

dos1 commented Oct 8, 2017

I initially wanted to make a PR that just generalizes the styles to be applied by classes instead of IDs, so adding custom CSS for that would be easy, but then I thought "hey, it could be just a matter of toggling the class name", so added an option for it. Take notice of how this doesn't actually add many new styles - if compact pinned tabs are disabled, the styles for regular tabs are used (and if they're compact, then the styles are just overriden). The rest is just fine tuning or making the stylesheet more elegant. Thanks to that, you could for instance easily apply the same styles as for pinned tabs for every other tab as well just by toggling the class name ;)

@eoger eoger force-pushed the master branch 10 times, most recently from 6c94073 to 10f5245 Compare October 22, 2017 19:31
@dos1
Copy link
Contributor Author

dos1 commented Oct 22, 2017

@eoger: Just noticed the 723b42e commit. Looks great, thank you!

I can see some minor background color tweaks to be made there, but let me PR it separately :)

@dos1 dos1 closed this Oct 22, 2017
@nirfse
Copy link

nirfse commented Oct 23, 2017

Just came here to say "thanks" for the feature I was waiting for!

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

Successfully merging this pull request may close these issues.

3 participants