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

fix(docs): alert docs - alert management import clarification #470

Merged

Conversation

bc-maksym-konohorov
Copy link
Contributor

Slightly changed docs for Alerts page.

Added some information regarding how to use createAlertsManager instance:
Screen Shot 2020-10-28 at 15 31 21

Changed code in CodeSnippet to be string, as it didn't render correctly, in was rendering extra } character, which could confuse consumers of this example:
Screen Shot 2020-10-28 at 15 30 53

Copy link
Member

@jdschmitt jdschmitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great update @bc-maksym-konohorov. As a consumer of BigDesign, this does make it more clear for me. Thanks for taking the time to improve our docs!

}}

{/* jsx-to-string:end */}
{`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should prob investigate the extra } you are mentioning.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My suggestion - jsx-to-string transformer works in a weird way, it depends on indentation in code, I moved two closing curly braces }} back, and it rendered it right. but there was a problem when you add just regular js code line before function. it rendered it incorrectly, and also linter on commit hook was breaking formatting.

</Text>

<CodeSnippet>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should add an example on how to create a useAlerts hook.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree it's a good idea, but I need to play with hook to become familiar with it, and unfortunately I'm not sure If I will have time for that in this sprint.

@bc-maksym-konohorov bc-maksym-konohorov merged commit 8a5e330 into bigcommerce:master Oct 28, 2020
@bc-maksym-konohorov bc-maksym-konohorov deleted the docs-alert-fix branch October 28, 2020 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants