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

DOC Update history viewer GraphQL examples for CMS 5 #196

Conversation

GuySartorelli
Copy link
Member

@GuySartorelli GuySartorelli commented Mar 26, 2023

Following on from #211, removes mention of GraphQL v3 and updates javascript to work with the CMS 5 dependencies.

Issue

@GuySartorelli GuySartorelli force-pushed the pulls/5.0/graphql-examples branch 2 times, most recently from 744c10c to 4dcf177 Compare March 27, 2023 00:12
onmatch() {
ReactDOM.render(
const root = createRoot(this[0]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just question, what do we have in this[0] exactly? Probably we could add a short comment to explain this in code snippet.

Copy link
Member Author

Choose a reason for hiding this comment

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

this[0] is a fairly common way in jQuery to say "Give me the underlying node that this jQuery object represents so I can use it with non-jQuery code". I'll add a comment when you've done with your review and assigned it back.

Copy link
Member Author

Choose a reason for hiding this comment

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

This part is no longer in this PR - see #247 instead.

@sabina-talipova
Copy link
Contributor

Looks good. Could you add comments for this, please.

@GuySartorelli GuySartorelli force-pushed the pulls/5.0/graphql-examples branch from 4dcf177 to 2d584b3 Compare April 12, 2023 00:40
@GuySartorelli GuySartorelli changed the title DOC Update GraphQL examples for CMS 5 DOC Update history viewer GraphQL examples for CMS 5 Apr 12, 2023
@GuySartorelli GuySartorelli marked this pull request as draft April 12, 2023 04:16
@GuySartorelli GuySartorelli force-pushed the pulls/5.0/graphql-examples branch from 2d584b3 to 77ce46d Compare April 17, 2023 22:57
@GuySartorelli GuySartorelli marked this pull request as ready for review April 17, 2023 22:57
@GuySartorelli
Copy link
Member Author

@sabina-talipova Do you remember what comments you wanted on this?

@sabina-talipova
Copy link
Contributor

@sabina-talipova Do you remember what comments you wanted on this?

In this comment you mentioned that you will update comment for this. #196 (comment)

Copy link
Contributor

@sabina-talipova sabina-talipova left a comment

Choose a reason for hiding this comment

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

LGFM. Tested locally. Provided example works.

@sabina-talipova sabina-talipova merged commit 96f222d into silverstripe:5.0 Apr 20, 2023
@sabina-talipova sabina-talipova deleted the pulls/5.0/graphql-examples branch April 20, 2023 22:27
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.

2 participants