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

auto-hide-sidebar-treestyletabs.css #101

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Conversation

radamar
Copy link

@radamar radamar commented Apr 5, 2018

The original auto-hide-sidebar.css from img2tab didn't work for me somehow. But this one did from TanzNukeTerror, I found this one to be a really useful mod that get's a lot right in a single package.
What is working:

  1. The sidebar is working smooth without any glitches for weeks now.
  2. The tabs have proper indents to show the parent-child hierarchy. Helps to map the tabs in your brain.

A slight modification allowed it to autohide other sidebars too, and it looks fantastic combined with ShadowFox

Copy link
Owner

@Timvde Timvde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like both! I just have two remarks, both of which are easy to solve :)

sidebar/auto-hide-sidebar-tst.css Outdated Show resolved Hide resolved
activitystream/hide-statuspanel.css Outdated Show resolved Hide resolved
@Timvde
Copy link
Owner

Timvde commented Apr 9, 2018

While I was rebasing and merging your PR (I like a clean git history :) ), I wanted to rename your auto-hide-sidebar-tst.css to auto-hide-sidebar.css (because you stripped out the TST-specific part)... and noticed that it already exists. Does your style have the same purpose now as the existing file?

@radamar
Copy link
Author

radamar commented Apr 9, 2018

Does your style have the same purpose now as the existing file?

Yeah. I think this one is much better.
mouse away from sidebar | mouse hover

@radamar
Copy link
Author

radamar commented Apr 9, 2018

wait, this one doesn't work for sidebar on the right side.

EDIT: It does!

Copy link
Author

@radamar radamar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with all the suggestions and have followed your review requests. Also, the autohide functionality is really the same as the old autohide.

activitystream/hide-statuspanel.css Outdated Show resolved Hide resolved
sidebar/auto-hide-sidebar-tst.css Outdated Show resolved Hide resolved
Copy link
Author

@radamar radamar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have followed through with the suggestions:

  1. Making the autohide feature generic
  2. renaming to sidebar/auto-hide-sidebar-tst.css
    It's your wish whether you want to keep the old 'auto-hide-sidebar.css' or replace it.

@radamar
Copy link
Author

radamar commented Sep 16, 2018

Hi, I would like to get this merged :)

@Timvde
Copy link
Owner

Timvde commented Sep 17, 2018

Hmm, it doesn't look like your screenshot for me on Linux/KDE: https://i.imgur.com/OtAXLL0.png

Do I need to also apply the "remove sidebar header" CSS maybe? (Since I don't see the header in your screenshot)

@radamar
Copy link
Author

radamar commented Sep 18, 2018

That's how it should look, you can tweak the --thin-tab-width: 35px; if you want deeper trees to be visible. I have it at 35px for 1920x1080 screen.

@Timvde
Copy link
Owner

Timvde commented Sep 18, 2018

But it's not even because I'm using nested tabs, it's the top level tabs.

@radamar
Copy link
Author

radamar commented Sep 18, 2018

try adding #sidebar { margin-left: -12 px !important }. I personally like the icons to be hidden partially or fully for less visual clutter. :-)

@Timvde
Copy link
Owner

Timvde commented Sep 18, 2018

It's just that it doesn't match the screenshot you provided... How does it look in Windows/MacOS (whichever platform you're on)? Can I just add that line unconditionally?

@radamar
Copy link
Author

radamar commented Sep 18, 2018

I am on GNU/Linux. I've tried it on windows once, looked the same. Maybe because I was trying out other things at that time. This is how it looks without margin-left: -12px https://ibb.co/jBx7We

@Timvde
Copy link
Owner

Timvde commented Sep 18, 2018

Would you be okay with me adding the margin-left: -12px by default? I think it's significantly better.

However, when hovering, I get a 12px white border at the right. I have toyed with it, but as I don't really know CSS, I couldn't get rid of it :(

@radamar
Copy link
Author

radamar commented Sep 20, 2018

that is a problem, thanks for pointing it out. try adding #sidebar-box { margin-left:-12px!important; }. And remove #sidebar { margin-left:-12px!important; } :-)

@radamar
Copy link
Author

radamar commented Sep 20, 2018

if it works, you can add it unconditionally :)

@Timvde
Copy link
Owner

Timvde commented Sep 21, 2018

That makes the sidebar un-hovered actually smaller. I think I managed to get what I want with these rules:

#sidebar {
  margin-left: -12px !important;
}

#sidebar:hover {
  margin-left: 0px !important;
}

This does make it show the first letter of the tab (https://i.imgur.com/nWfejXG.png), and there is a small animation where icons move back to the right when hovering. Is that okay for you?

@radamar
Copy link
Author

radamar commented Sep 22, 2018

I think these sorts of changes belong in Tree style tabs's css or any other sidebar's settings. userChrome.css tweaks will also affect history and bookmarks sidebar. margin-left was already a hack, let's not make it worse by adding more hacks on top of it, just my 2 cents :)
Here is how it looks after adding the appropriate tweaks from TST wiki1 : https://ibb.co/cfTNNU

@Timvde
Copy link
Owner

Timvde commented Sep 22, 2018

This is the specific -tst style, and the changes are not to tst's own code. So is it okay for you to merge with the two styles I mentioned in my last comment applied? I do think it's better than not having them, but if you disagree, I'll merge it as is.

@radamar
Copy link
Author

radamar commented Sep 22, 2018

Please merge as is :). You can even replace the autohide-sidebar but I guess that might break things for some people. Thanks!

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.

2 participants