sindresorhus / hide-files-on-github

Chrome extension - Hide nonessential files from the GitHub file browser
https://chrome.google.com/webstore/detail/hide-files-on-github/lpnakhpaodhdkleejaehlapdhbgjbddp
MIT License
320 stars 36 forks source link

Move toggle button to file table. Closes #33 #34

Closed radiovisual closed 8 years ago

radiovisual commented 8 years ago

Essentially everything works as it did before, only now the button has moved from the File Menu Bar, to the bottom of the Files table.

The new "hidden view":

show-dotfiles

The new "expanded view":

hide-dotfiles

When the new toggle button is hovered, it takes on the blue color of the other file links in the files table.

radiovisual commented 8 years ago

I have a slight fade effect on the Show dotfiles toggle button, but now that I am using it, I feel it might be a bit too gimmicky, if you agree, I will remove the fade effect.

sindresorhus commented 8 years ago

Generally looks good :)

Yeah, drop the fade. Also noticed the top border doesn't fill the full width. You probably need to make some empty columns to match the other rows.

radiovisual commented 8 years ago

Ah, right. Ok, I removed the fade, and ensured the appropriate length of the tr by dropping in empty td tags to match the same td "signature" as the other rows in the files table.

column-width

sindresorhus commented 8 years ago

Noticed another bug. When in a sub-folder it will show the button if even when no files where hidden: https://github.com/avajs/ava/tree/master/bench I'm aware this is an existing bug, but it's more confusing now that it's inline in the table.

radiovisual commented 8 years ago

Hm. Maybe we can either check for the presence of dotfiles, and only show the toggle button if dotfiles exist, or it might be easier to check the table.files table for the tr.up-tree class, and if the up-tree class is found, don't draw the toggle button (because the up-tree class only appears when you are in a folder view).

radiovisual commented 8 years ago

It solves the problem to check for the existence of 'tr.up-tree'which I confirmed in my latest commit if you want to check it out. This approach will only render the button when you are viewing files on the main file view.

sindresorhus commented 8 years ago

LGTM

@jamestalmage ?

radiovisual commented 8 years ago

I tweaked the toggle button to act more like a link. Before, the entire row was clickable, and as a result, the :hover state would fire whenever I would scroll down to read a project's readme.md, which was a bit annoying, so now the behavior treats the toggle button more like an actual link, which focuses the behavior appropriately.

jamestalmage commented 8 years ago

:shipit:

radiovisual commented 8 years ago

There is still technically a bug when you enter a project directory. The "check for tr.up-tree trick" works well to hide the toggle button when you are entering a directory, but the extension still hides the dotfiles inside the directory, without a way to show/hide them. You can see what I mean by noticing how the .babelrc and .alt-babelrc files in ava/test/fixture/babelrc are un-viewable when the extension is enabled.

So perhaps it would be better to check for the existence of dotfiles, and only show the toggle button when dotfiles exist in the directory, OR we can keep the behavior we have now, and simply disable the extension when you enter a directory, so that the toggle button only shows up on the root file view, and lets you see all files normally when you enter a directory.

Or a third option, a combination of both: the toggle button doesn't even need to show up on repos that don't have any dotfiles, even on the root file view, so the toggle button can also act like a reminder that there are dotfiles in the current directory view, and they are either "hidden" or "visible".

radiovisual commented 8 years ago

So I have updated the code to disable the extension when you are not in a directory view. This fixes the bug, because this is essentially what should be happening when we hide the toggle button from view in the directories (without a toggle button, there should be no toggling of files), but it still leaves the question: do we want the extension active when inside of a directory?

jamestalmage commented 8 years ago

I really only care about it in the base of the project. In fact, if dotfiles are present inside a subdirectory, that's really unusual - and I probably don't want that obfuscated.

So I say disable for every directory but the root one.

radiovisual commented 8 years ago

I agree. If a dotfile is in a subdirectory, then I usually want to know about it. So the current commit I made means the extension is ready to be shipped, since my latest commit ensures that the extension only works on the root file view.

sindresorhus commented 8 years ago

Agreed. I too only care about the top-level view.

radiovisual commented 8 years ago

Ok, cool. Then this PR is ready to merge.

sindresorhus commented 8 years ago

Thank you @radiovisual. Much better than the previous :)