Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make styles look like GitHub's #43

Merged
merged 1 commit into from
May 15, 2014
Merged

Conversation

athaeryn
Copy link
Contributor

This project is awesome! I've done a bit of a design overhaul in order to make the sidebar look like it's part of GitHub.

Most notably, I've used GitHub's icon font for the directory and file icons.

Feel free to question anything I've done. :)

@buunguyen
Copy link
Collaborator

Hi, thanks for this. GitHub can't merge this automatically, I think it's because master branch contains many changes from the last time you pulled. Can you please pull the latest and submit a PR on top of it? It's easier for me to review, test and merge. Thank you.

@athaeryn
Copy link
Contributor Author

Done!

@athaeryn
Copy link
Contributor Author

By the way, this should handle issues #10, #30, and #33. :)

@buunguyen
Copy link
Collaborator

This is amazing, I totally love it! Thank you!
I think it just needs a little bit more tweaking before merging:

Chrome: there are 2 issues, as below. The first issue is visible when you expand a repo with deep paths (e.g. Angular UI). The second issue is not really a big deal, but I think the previous button background makes it looks better when sitting on text. I'm open to suggestion.
issues_chrome

Safari: the h1 box is too big. This happens with existing style as well, I hope you can address it in your style. I've packaged the Safari extension here with the new styling, so that you can try (just drag to Safari) without having to register for Safari developer certificate.
issues_safari

Finally, I sort of missing the ability to know which file or folder on the tree is being selected. Guess I want close-to-IDE experience. Is there a way to bring it back while blending it seamlessly to the new styling?

@buunguyen
Copy link
Collaborator

@athaeryn someone on Reddit suggests this design, I think moving the button to the top looks neat and addresses its issue when sitting on top of text:

Thought?

@calumk
Copy link

calumk commented May 14, 2014

Hey Im the "someone from reddit"
You can find my code here:
https://github.com/calumk/octotree

only the two css files have changed.
inject.css and style.css
I also modified the 32px file.

@buunguyen
Copy link
Collaborator

@calumk: thanks for stopping by and sharing the code. It's a neat design you made. I hope @athaeryn can take a look and make a PR soon.

@athaeryn
Copy link
Contributor Author

Oh, that works. Nice! I'm putting the finishing touches on some more styling goodness, should be done soon. :)

@athaeryn
Copy link
Contributor Author

Alright, I'm done for now. When things look good I can squash all these commits.

@buunguyen
Copy link
Collaborator

@athaeryn This is really really slick! I'm don't quite like the separator and increased height for the nodes though. I'm looking at a large repo (AngularJS) and the tree is too tall. I'd prefer the height and no-separator style of your original design. Other than that, everything looks great and I'm very eager to merge this pull request.

@athaeryn
Copy link
Contributor Author

Thanks! :)

That's fair. I was trying to match the GitHub file view, but since they're different views they don't have to be that similar.

I'm going to be offline for an hour or so, but I'll play around with decreasing the height when I can.

Tweak styles

Make text overflow ellipsis work on repo name

Fix transition so button stays with panel

Reduce height of nodes
@athaeryn
Copy link
Contributor Author

I've reduced the height a bit, squashed the commits together, and rebased on the latest from master. 👍

buunguyen added a commit that referenced this pull request May 15, 2014
Make styles look like GitHub's
@buunguyen buunguyen merged commit 4f676a6 into ovity:master May 15, 2014
@athaeryn athaeryn deleted the github-styles branch May 15, 2014 04:14
@buunguyen
Copy link
Collaborator

Merged. I absolutely adore the new look! Excellent job and thanks again for working on this, Mike!

@isbadawi
Copy link
Contributor

Awesome! I'm excited to use this -- I'd suggest pushing an update to the Chrome store now that this is in.

@buunguyen
Copy link
Collaborator

Hopefully I can push to the store this evening and everyone can enjoy @athaeryn's design by tomorrow!

@athaeryn
Copy link
Contributor Author

I'm really glad you enjoy it!

I can't take all of the credit though. Putting the toggle button on top was @calumk's idea, and a lot of the styles are just copying GitHub's.

I'm sure I'll be contributing to this project in other areas, it's seriously great.

@calumk
Copy link

calumk commented May 15, 2014

Well done all!
Isn't collaboration fun :)
Looking forward to installing the new version

@buunguyen
Copy link
Collaborator

The new release has arrived to the Chrome store. Thanks again for your work, @athaeryn and @calumk!

This was referenced May 15, 2014
Closed
@buunguyen buunguyen added this to the 1.1 milestone May 19, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants