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

Replace React integration demo with proper documentation #73

Merged
merged 1 commit into from
Oct 7, 2016

Conversation

tehapo
Copy link
Contributor

@tehapo tehapo commented Sep 29, 2016

Fixes vaadin/components-team-tasks#210


This change is Reviewable

@Saulis
Copy link
Contributor

Saulis commented Oct 3, 2016

:lgtm: thx @tehapo ! Stay awesome! ping @magi42 for review.


Review status: 0 of 9 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@magi42
Copy link

magi42 commented Oct 5, 2016

Reviewed 9 of 9 files at r1.
Review status: all files reviewed at latest revision, 7 unresolved discussions.


docs/integrations/integrations-react.adoc, line 2 at r1 (raw file):

---
title: Overview

This is the title shown in the menu, it should match the article title below (can be shortened in the menu)


docs/integrations/integrations-react.adoc, line 9 at r1 (raw file):

# React Integration

The link:https://facebook.github.io/react/[React] library from Facebook integrates easily with web components.

docs/integrations/integrations-react.adoc, line 14 at r1 (raw file):

ifdef::web[]
====
See also the link:https://facebook.github.io/react/docs/webcomponents.html[Web Components page] within React documentation.
  • within -> in

docs/integrations/integrations-react.adoc, line 25 at r1 (raw file):

image::img/react-integration.png[]

The example consists of two React components, `UserApp` and `UserGrid`.
  • Lists should be introduced with colon (:), not with comma
  • The React components look like they follow naming convention for classes. Maybe [classname] style should be used for them?

docs/integrations/integrations-react.adoc, line 27 at r1 (raw file):

The example consists of two React components, `UserApp` and `UserGrid`.
The `UserGrid` component wraps the [vaadinelement]#vaadin-grid# element and does all necessary initialization to display a list of random users.
As React doesn't support link:https://facebook.github.io/react/docs/jsx-gotchas.html#custom-html-attributes[custom attributes] on standard elements, [vaadinelement]#vaadin-grid# DOM API can't be fully utilized in the initialization.
  • Don't use contractions: doesn't -> does not, can't -> cannot

docs/integrations/integrations-react.adoc, line 28 at r1 (raw file):

The `UserGrid` component wraps the [vaadinelement]#vaadin-grid# element and does all necessary initialization to display a list of random users.
As React doesn't support link:https://facebook.github.io/react/docs/jsx-gotchas.html#custom-html-attributes[custom attributes] on standard elements, [vaadinelement]#vaadin-grid# DOM API can't be fully utilized in the initialization.
Fortunately [vaadinelement]#vaadin-grid# also provides corresponding JavaScript APIs.

Comma after sentence adverb: "Fortunately, "


docs/integrations/integrations-react.adoc, line 35 at r1 (raw file):

The code below can be run and forked as a JSFiddle at https://jsfiddle.net/pw1nLaL8/2/.

```javascript
  • ````` is for monotext, but this is a source code listing and you want syntax highlighting, so use:
  [source, JavaScript]
  ----
  ...
  ----

Comments from Reviewable

@Saulis
Copy link
Contributor

Saulis commented Oct 6, 2016

Review status: all files reviewed at latest revision, 7 unresolved discussions.


docs/integrations/integrations-react.adoc, line 2 at r1 (raw file):

Previously, magi42 (Marko Grönroos) wrote…

This is the title shown in the menu, it should match the article title below (can be shortened in the menu)

Done.

docs/integrations/integrations-react.adoc, line 9 at r1 (raw file):

Previously, magi42 (Marko Grönroos) wrote…

Done.

docs/integrations/integrations-react.adoc, line 14 at r1 (raw file):

Previously, magi42 (Marko Grönroos) wrote…

  • within -> in
Done.

docs/integrations/integrations-react.adoc, line 25 at r1 (raw file):

Previously, magi42 (Marko Grönroos) wrote…

  • Lists should be introduced with colon (:), not with comma
  • The React components look like they follow naming convention for classes. Maybe [classname] style should be used for them?
Done.

docs/integrations/integrations-react.adoc, line 27 at r1 (raw file):

Previously, magi42 (Marko Grönroos) wrote…

  • Don't use contractions: doesn't -> does not, can't -> cannot
Done. Note taken.

docs/integrations/integrations-react.adoc, line 28 at r1 (raw file):

Previously, magi42 (Marko Grönroos) wrote…

Comma after sentence adverb: "Fortunately, "

Done.

docs/integrations/integrations-react.adoc, line 35 at r1 (raw file):

Previously, magi42 (Marko Grönroos) wrote…

  • ````` is for monotext, but this is a source code listing and you want syntax highlighting, so use:
  [source, JavaScript]
  ----
  ...
  ----
Done.

Comments from Reviewable

@Saulis Saulis merged commit 7c1284f into master Oct 7, 2016
@Saulis Saulis deleted the react-docs branch October 7, 2016 11:06
@Saulis Saulis removed the in review label Oct 7, 2016
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