Skip to content

Add Gitter button#227

Merged
marcustisater merged 2 commits intopostcss:masterfrom
natkaratkova:add-gitter-button
Jan 8, 2017
Merged

Add Gitter button#227
marcustisater merged 2 commits intopostcss:masterfrom
natkaratkova:add-gitter-button

Conversation

@natkaratkova
Copy link
Copy Markdown
Contributor

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

@ai
Copy link
Copy Markdown
Member

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
Copy Markdown
Member

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
Copy Markdown
Member

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
Copy Markdown
Contributor Author

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
Copy Markdown
Member

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
Copy Markdown
Member

ai commented Jan 8, 2017

@meuwka great! Thanks for you work :).

@marcustisater ping me when you deploy it :)

@natkaratkova
Copy link
Copy Markdown
Contributor Author

@ai thanks :)

@marcustisater
Copy link
Copy Markdown
Member

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
@marcustisater
Copy link
Copy Markdown
Member

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

@natkaratkova natkaratkova deleted the add-gitter-button branch January 10, 2017 10:20
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.

3 participants