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 35 forks source link

Leave pull request label unchanged #28

Closed clayreimann closed 8 years ago

SamVerschueren commented 8 years ago

Why?

clayreimann commented 8 years ago

It was a controversial-ish change I made in #27, but I think that this reverts the issue I fixed in that PR too. For now don't merge.

radiovisual commented 8 years ago

I am not completely for or against, but if I had to choose, I like the idea of shortening the lesser-used elements to keep the "New Pull Request" button unchanged. I think it looks better that way, and some of the lesser-used UI elements can spare some room if we need it.

sindresorhus commented 8 years ago

This still applies: https://github.com/sindresorhus/hide-files-on-github/pull/27#issuecomment-196757357

SamVerschueren commented 8 years ago

With what element are you going to gain some extra space in order to keep the original New Pull Request text? I'm fine with how it is at the moment. Everyone know where PR stands for, right?

You might win some space if you could use icons for New File, Upload files, etc. But I think that will make things even worse.

radiovisual commented 8 years ago

Simply changing the "Download ZIP" button to "Download" makes enough room from the "New Pull Request" button to remain unchanged, even when you are on a long branch name. In the screenshot below, I am on the "connectedness" branch of Browserify:

demo

The only change that had to take place was changing the "Download ZIP" button to "Download".

I do think things look better this way, but like I said before, I am neither for or against, so keeping "New PR" is ok too.

radiovisual commented 8 years ago

Here is another example where I am demoing the same concept, only with a much longer branch name. The branch name used in the screenshot below is: some-super-long-branch-name-that-is-super-super-long, yet everything still fits when you remove the "ZIP" from "Download ZIP":

demo2

This works because github truncates the branch name and only adds a few extra ellipsis characters (...) for branch names longer than 6 characters (i.e, master), so we only need a few extra pixels of screen space to fit the "New Pull Request" button alongside a branch name other than master, and changing the "Download ZIP" button to "Download" gives us more than enough room to fit everything.

SamVerschueren commented 8 years ago

I'm not sure about removing Zip entirely from the Download Zip button. It's not clear in what format the project is being downloaded, although one can guess it's a zip file. But it feels like it misses valuable information while PR is just an abbreviation of Pull Request.

I'm ok with either one of them, just tossing in some arguments :).

radiovisual commented 8 years ago

Yeah, you're right. Leaving in the "ZIP" makes the Download button more semantic. So here is another alternative: make the input text field with the repo url in it shorter. What you see by default isn't very helpful: https://gith, and tells the same story as https://, and since this input field gets it's very own "copy to clipboard" button, you rarely ever need to read the full text in that input (especially since you can just look up into your browser's address bar to see the same url). So here is a screenshot where the "Download ZIP" button remains unchanged, but the repo url field is shortened from 200px to 179px:

demo3

Using:

.file-navigation-option .input-group {
    width: 179px;
}

.file-navigation-option .input-group input[type="text"] {
    width: 71px;
}
clayreimann commented 8 years ago

I agree with the need to leave ZIP so I changed Download. I also tried to hide the git url text input completely, but it would appear that the copy mechanism doesn't work if the text isn't actually visible on screen (so it breaks when the input < ~20px).

I was thinking along these lines:

screen shot 2016-03-17 at 8 20 15 am

I know it's a bit aggressive but it would be nice to be somewhat isolated from minor style tweaks github may make in the future.

radiovisual commented 8 years ago

I don't think that being that aggressive is necessary. In my opinion, it weakens the aesthetic of the UI. While it would be nice to future-proof the layout against any UI changes that Github might make in the future, I think that your "Get ZIP" alternative works nicely by itself, without making any other changes, and it adds plenty of room on the UI for Github to make changes in the future.

Here is a sample of just Changing "Download ZIP" to "Get ZIP", and it looks beautiful:

demo4

radiovisual commented 8 years ago

You can save even more pixel space by updating the "Download ZIP" button to use an icon:

image

This option makes plenty of room for future UI updates, and only requires one change to the UI, and still looks pretty.

sindresorhus commented 8 years ago

Could use https://octicons.github.com/icon/file-zip/

radiovisual commented 8 years ago

I think that is my favorite option: using the icon that Sindre linked to for the "Download ZIP" button, and leaving everything else as-is.

clayreimann commented 8 years ago

Bundling Octicons for just for the ZIP button seems a bit over the top to me. I reverted most of the changes.

image

The download button has a down-arrow instead of the word "Download" and I tweak the git clone url box a bit.

radiovisual commented 8 years ago

@clayreimann , you wouldn't have to bundle the octicons with the extension, because the octicons are already loaded onto the github page. All you have to do is add the appropriate class for the zip icon.

The "Copy to Clipboard" and "Save to Desktop" icons that are sitting right next to the Download button are calling to the octicons class, so you can use those as reference.

radiovisual commented 8 years ago

Or, you can just use the github/octicons/file-zip.svg individually. Just like github does on their own pages.

Using this:

downloadBtn.innerHTML = '<svg height="14" width="16" class="octicon" viewBox="0 0 16 16" role="img" xmlns="http://www.w3.org/2000/svg"><path d="M8.5 1H1C0.45 1 0 1.45 0 2v12c0 0.55 0.45 1 1 1h10c0.55 0 1-0.45 1-1V4.5L8.5 1z m2.5 13H1V2h3v1h1v-1h3l3 3v9zM5 4v-1h1v1h-1z m-1 0h1v1h-1v-1z m1 2v-1h1v1h-1z m-1 0h1v1h-1v-1z m1 2v-1h1v1h-1z m-1 1.28c-0.59 0.34-1 0.98-1 1.72v1h4v-1c0-1.11-0.89-2-2-2v-1h-1v1.28z m2 0.72v1H4v-1h2z" /></svg> ZIP';

Results in:

demo6

clayreimann commented 8 years ago

Ultimately decided to keep ZIP in the button to be explicit about what is getting downloaded. Unless there are strenuous objections this is what my PR will look like.

image

sindresorhus commented 8 years ago

Thank you @clayreimann :)