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

void 0 #8

Merged
merged 1 commit into from Dec 5, 2016
Merged

void 0 #8

merged 1 commit into from Dec 5, 2016

Conversation

ghost
Copy link

@ghost ghost commented Dec 5, 2016

The bookmarklet needs a

void 0

on the end else it fails

@FarhadG
Copy link
Owner

FarhadG commented Dec 5, 2016

Thanks for opening the issue, @svnpenn. Can you please elaborate what you mean? What breaks and what needs this additional void 0? Thank you!

@ghost
Copy link
Author

ghost commented Dec 5, 2016

void 0 prevents the bookmarklet from redirecting the page, else you are left with

[object Promise]

for goodness sake, it even says it on wikipedia

http://wikipedia.org/wiki/Bookmarklet#Concept

@FarhadG
Copy link
Owner

FarhadG commented Dec 5, 2016

Ha, thanks for the link. It makes sense... it wasn't doing that in my browser but can see why it's needed in our case.

For goodness sake, can you open a PR, @svnpenn ? :)

@ghost
Copy link
Author

ghost commented Dec 5, 2016

@FarhadG done

@FarhadG
Copy link
Owner

FarhadG commented Dec 5, 2016

Great! Thanks, @svnpenn !

@FarhadG FarhadG merged commit 983e3d6 into FarhadG:master Dec 5, 2016
@ghost
Copy link
Author

ghost commented Dec 5, 2016

@FarhadG it looks like I missed landing/index.html - do you need me to send another PR or can you get it?

@FarhadG
Copy link
Owner

FarhadG commented Dec 5, 2016

Oh, good catch, @svnpenn! You should just run the build script as a previous PR was put in to be generate that for us. Thanks for improving this for everyone.

@ghost
Copy link
Author

ghost commented Dec 5, 2016

@FarhadG
Copy link
Owner

FarhadG commented Dec 6, 2016

@svnpenn , it's because the master branch is what's used for development and gh-pages is the same thing but the one that github pages tracks for the static site. Last time I checked, you needed the gh-pages branch for the static sites. I assume it's still the case.

@ghost
Copy link
Author

ghost commented Dec 6, 2016

@FarhadG you do not need both. You can do:

  1. Just the gh-pages branch, see http://github.com/svnpenn/bm

  2. Just the master branch

  3. Just the docs folder of the master branch, see
    http://github.com/svnpenn/rosso

@FarhadG
Copy link
Owner

FarhadG commented Dec 6, 2016

Awesome! It's been some time since I've looked at what they do now. Good to know. Thanks for the info, @svnpenn . Feel free to open a PR and I'll merge -- I like the demo version.

FarhadG added a commit that referenced this pull request Dec 6, 2016
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.

1 participant