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

Options grid update and cleanup #423

Closed
wants to merge 4 commits into from
Closed

Options grid update and cleanup #423

wants to merge 4 commits into from

Conversation

tiesont
Copy link
Member

@tiesont tiesont commented Apr 17, 2015

Added 'value' to options grid.

Also did a little cleanup on options grid to space out content a bit.

Added CSS rules to initially hide sub-navs on API sidebar.

Also did a little cleanup on options grid to space out content a bit.

Added CSS rules to initially hide sub-navs on API sidebar.
This should have been deleted from my branch when I first forked it; makeusabrew owns that CNAME.
@makeusabrew
Copy link
Collaborator

Sorry, been stacked out with work for the past few weeks. Only comment is the removal of the CNAME file - I don't want to be merging that! :)

@tiesont
Copy link
Member Author

tiesont commented May 4, 2015

The CNAME bit should just be for my fork. The pages builder keeps trying to
use it when I push content, so I removed it.
On May 4, 2015 5:25 AM, "Nick Payne" notifications@github.com wrote:

Sorry, been stacked out with work for the past few weeks. Only comment is
the removal of the CNAME file - I don't want to be merging that! :)


Reply to this email directly or view it on GitHub
#423 (comment).

@tarlepp
Copy link
Collaborator

tarlepp commented May 4, 2015

You could always add that CNAME to .gitignore.

@tiesont
Copy link
Member Author

tiesont commented May 4, 2015

Those IDs are for linking the sidebar nav, so I don't see how classes would
be relevant. Am I missing something, otherwise?

On Mon, May 4, 2015 at 4:39 PM, Tarmo Leppänen notifications@github.com
wrote:

Also I'm not liking that you're adding same id attribute those
elements, id should be unique. So on this case you should use class
definition instead.


Reply to this email directly or view it on GitHub
#423 (comment).

Tieson Trowbridge
Software Developer, Arivium Inc
arivium.com

@tarlepp
Copy link
Collaborator

tarlepp commented May 5, 2015

@tiesont I was wrong, was looking this PR with my phone with a bit of hurry... Sorry for that wrong comment.

@tiesont
Copy link
Member Author

tiesont commented May 5, 2015

@tarlepp No need to apologize, I just wanted to make sure I hadn't missed something obvious. I do need to get better at understanding Git; I thought the pull-request would only include what was committed at the time of the request, but it's obviously including bits like me removing the CNAME file from my fork, which happened afterwards...

@makeusabrew If it's easier, I can close/delete this pull request, do a fresh fork, and then commit the changes I intended to be part of the original pull request.

@makeusabrew
Copy link
Collaborator

@tiesont up to you; totally understand why you'd have deleted the CNAME file if GitHub was moaning at you, but as the PR stands merging it would break bootboxjs.com 😢

You can either revert that commit (git revert 87304d3) or as you say, go for a fresh fork. Whichever you prefer :)

This reverts commit 87304d3.
@tiesont
Copy link
Member Author

tiesont commented May 6, 2015

@makeusabrew Revert was probably the easiest option... :)

@tiesont tiesont closed this Jun 19, 2015
@tiesont tiesont deleted the gh-pages branch June 19, 2015 07:37
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.

None yet

3 participants