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

Add Gitter button #227

Merged
merged 2 commits into from Jan 8, 2017
Merged

Conversation

@natkaratkova
Copy link
Contributor

@natkaratkova natkaratkova commented Jan 8, 2017

Add code from https://sidecar.gitter.im/ through react-helmet

@ai
Copy link
Member

@ai ai commented Jan 8, 2017

@meuwka Thanks! Should we set async and defer to true (at least official docs tell about it, but I not sure will it works with React)?

@ai
Copy link
Member

@ai ai commented Jan 8, 2017

@meuwka Travis CI reported about SyntaxError: Unexpected token .. Do you have it locally? Or it is problem in tests?

@ai
Copy link
Member

@ai ai commented Jan 8, 2017

Other tricky question. Should we change default color of this button?

@meuwka how it looks right now? Is it green as in official docs? Is it hard to change it to brand red (#dd3a0a)?

@marcustisater do you think that brand red will be the best for this button?

@natkaratkova
Copy link
Contributor Author

@natkaratkova natkaratkova commented Jan 8, 2017

@ai Hi! Travis CI reported this issue in master also. Problem is in highlight.js file

I'm not sure about defer (old browsers) but 'async' probably we should use because script is embedding in <head > I set undefined value because it's way from docs react-helmet (when attribute takes no value)

Yea, button is green, we also can create our own button. Sidecar has activationElement in options. Or just set color

@ai
Copy link
Member

@ai ai commented Jan 8, 2017

Great :). I think we could merge even green version of it.

Let’s wait for @marcustisater review. I think it could take few hours. So if you have time you could make button red in current PR :).

@ai
Copy link
Member

@ai ai commented Jan 8, 2017

@meuwka great! Thanks for you work :).

@marcustisater ping me when you deploy it :)

@natkaratkova
Copy link
Contributor Author

@natkaratkova natkaratkova commented Jan 8, 2017

@ai thanks :)

@marcustisater
Copy link
Member

@marcustisater marcustisater commented Jan 8, 2017

You are correct, the error is from highlight.js and not from this PR. I will create separate issue for that.

This PR LGTM, good work @meuwka 🎉😄

@marcustisater marcustisater merged commit 9eda9a4 into postcss:master Jan 8, 2017
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@marcustisater
Copy link
Member

@marcustisater marcustisater commented Jan 8, 2017

@ai It's live now but appcache so it might take a while 😅

@natkaratkova natkaratkova deleted the natkaratkova:add-gitter-button branch Jan 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants