-
Notifications
You must be signed in to change notification settings - Fork 43
[PUB-1552] Add Pub/Sub React getting started guide #2534
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
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
92e5e98
to
a74cbf9
Compare
So there already exists content/getting-started/react.textile file which renders at https://ably.com/docs/getting-started/react. I couldn't straight up replace it with the new content as there are a bunch of places in the docs that reference that file:
And even the new Chat getting started guide for React has a reference to it: https://github.com/ably/docs/pull/2521/files#diff-7236fae23c5195699eddce94eb3f8c2f307b171b5f7035c85f974f86db02e9c7R77. By looking at these references, I can see that the existing React documentation is mostly used as an API reference for hooks and the The Chat getting started guide handles similar cases nicely - it provides links such as Currently, we don't publish ably-js React hooks to TypeDoc (see the latest TypeDoc for v2, which includes only @ttypic @m-hulbert wdyt? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this! Overall it looks really great. I have stepped through the guide myself and left some comments on where I think we could make the end to end journey a bit more seamless/fewer gotchas.
I do also wonder whether for this type of getting staerted guide it's worth having e.g. a repo in e.g. Ably labs that walks through each step commit by commit? cc @m-hulbert what do you think?
|
||
Once installed, remove any existing styles from your @src/index.css@ file and ensure that it contains only the Tailwind CSS import: | ||
|
||
```[css] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using [css]
here means that it doesn't play nicely with the language selector:
@m-hulbert do you have any suggestions on how to handle this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably fixed in #2545.
the base branch for this PR (https://github.com/ably/docs/tree/edu-1845-pubsub-js-start) would need to be rebased on latest main to see if that fixed an issue
that PR won't help in this case: we need to display a code block for css which will never match currently selected language for the docs
We used to do this for tutorials but they became unmaintainable. Do you think many people would go through commit-by-commit in this way? |
3fff3bd
to
2b46717
Compare
I also removed notes about ably-js 2.0 has been out for a ~year now, I don't think we should still have "new" and "updated" flags |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks brilliant, really well put together, super clear and easy to follow. Minor comment but approving ahead of those small fixes. Thank you for this!
|
||
h2(#next). Next steps | ||
|
||
Continue to explore the Ably Pub/Sub documentation with React as the selected language: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can remove this line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The next steps section is basically identical across getting started guides and includes this line. No big reason to remove it I think
80a3b87
to
d6aeb8a
Compare
2b46717
to
40623ea
Compare
@VeskeR @m-hulbert do we plan to do anything with this? |
@jamienewcomb Andrii was going to update to move to Tailwind to match with what we decided with Chat. I don't think he had the chance before vacation. |
@m-hulbert I can't see any acknowledgement that he is going to do that on this PR. is that a hard requirement? |
@jamienewcomb to keep them consistent, yes. There's a Slack thread where we've agreed this. |
e3755c9
to
33835a1
Compare
@m-hulbert went back to using Tailwind in 33835a1. There is last outstanding question regarding existing content/getting-started/react.textile file |
33835a1
to
b35cd82
Compare
b35cd82
to
7a0b321
Compare
af68b20
to
792fc22
Compare
I've made small changes to this getting started to bring things more inline with the existing JS getting started guide in terms of layout etc. I've also moved our react hooks page to I would advise we don't merge just yet as need to discuss with marketing and other depts to determine any other links that link to the react hooks page. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, but not to be merged today.
@GregHolmes we will leave this with you in that case and mark this ticket as done on our side if that's okay, thanks |
Resolves PUB-1552
Use raw css instead of tailwind, split components into separate files, and provide full contents for files that can be copied and pasted as is. Co-authored-by: Mike Christensen <[email protected]>
7602c8d
to
5006227
Compare
Description
This PR adds a new getting started guide for Ably Pub/Sub with React.
This getting started guide is intended to replace the existing one at https://ably.com/docs/getting-started/react, which can be found at content/getting-started/react.textile. Due to some issues explained in this comment currently this PR adds a new file
react-new.textile
for the Pub/Sub React getting started guide.Resolves PUB-1552
Checklist