-
Notifications
You must be signed in to change notification settings - Fork 180
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
Add information about shared component API #307
Conversation
@miq-bot assign @himdel |
5440bf7
to
61b6760
Compare
ui/register_react_component.md
Outdated
import addReact from 'helpers'; | ||
import { YourComponent } from 'your-components/custom-component'; | ||
|
||
addReact('YourComponentName', props => <YourComponent {...props} />); |
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.
why don't just call
addReact('YourComponentName', YourComponent);
?
ui/register_react_component.md
Outdated
@@ -0,0 +1,98 @@ | |||
### Register react component | |||
In ManageIq project there is a way of sharing components, getting their state and control their flow by something which is called ManageIq component. |
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.
ManageIQ, the q is big ;)
ui/register_react_component.md
Outdated
Instance's ID under which it's created. | ||
|
||
#### interact | ||
Should the component chooses it can supply object over which it's possible side channel communication. |
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.
s/chooses/choose/
which/that?
"possible side channel communication" -- verb missing?
This function helps you to update component's public props. You can add new prop or update existing prop. | ||
|
||
#### destroy | ||
To destroy instance and remove it from DOM (no point of return) simply call this function. |
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.
no way back?
ui/register_react_component.md
Outdated
``` | ||
|
||
### Component instance API | ||
When you create new instnace or search for instance of component you will receive object over which you can interact with this instance and observe it's properties. |
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.
s/instnace/instance
"which" sounds Czenglish
ui/register_react_component.md
Outdated
### Register react component | ||
In ManageIq project there is a way of sharing components, getting their state and control their flow by something which is called ManageIq component. | ||
|
||
This registry and usage consists of three steps, define blueprint which holds the react component with it's props. Then you will define it inside shared components (use blueprint created in step 1). And when someone decides to use such component he will call function newInstance with custom props and element to which such component is bound to. |
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.
it's = it is
its = possessive
... in many places :)
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.
s/which holds/that holds
ui/register_react_component.md
Outdated
|
||
When creating new instance you have option to pass multiple parameters: | ||
* name - component's name under which component was defined | ||
* props - default props to component |
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.
default props for the component
ui/register_react_component.md
Outdated
import newInstance from 'registry'; | ||
|
||
const props = {}; | ||
const element = document.getElementById('element-id'); |
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.
no getElementById, newInstance
should take a normal selector
(or should this be using componentMarkup
instead?)
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.
New instance expects mount to element, because we don't know how user gets such element either trough getElement, trough react prop or whatever. @Hyperkid123 is writing helper method which will be shortcut to these commands.
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.
What I'm saying is - the default method to instantiate a component must follow the same rules as all the other methods (like miq_bootstrap
).
So.. if this documents newInstance
as something you would want to use, we need to change it to take selectors.
If this documents componentMarkup
as the thing to use, and mentions newInstance
only for the cases where you're doing stuff manually, we don't need to change it.
But.. right now, it appears like newInstance
is the preferred way, possibly not what you intended?
ui/register_react_component.md
Outdated
``` | ||
|
||
#### 2. New instance | ||
When you want to create new instance of recently created component just call newInstance function with these arguments |
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.
newInstance
(backticks missing)
ui/register_react_component.md
Outdated
|
||
#### 2. New instance | ||
When you want to create new instance of recently created component just call newInstance function with these arguments | ||
* name - name of component you wish to use. |
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.
name of the component
ui/register_react_component.md
Outdated
#### 2. New instance | ||
When you want to create new instance of recently created component just call newInstance function with these arguments | ||
* name - name of component you wish to use. | ||
* props - object with props which will be mapped to your components (used when creating blueprint) |
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.
s/props which will/props that will/
Also this is missing any mention of |
This should probably also include a way to get redux running...
(None of that is true, but it needs to be documented ;).) |
@karelhala can you add the component factory usage? |
@himdel @martinpovolny thanks for the review 👍 . Fixed all of your comments and added section about import mapping so when we use ES5 we know where to look for. EDIT: It's at the bottom of documentation, should I add it above component API? Also component API is not fully covered some functions are missing, but the basic concept of defining and creating new instance, plus basic usage of component's API is covered. |
ui/register_react_component.md
Outdated
@@ -0,0 +1,107 @@ | |||
### Register react component | |||
In ManageIQ project there is a way of sharing components, getting their state and control their flow by something which is called ManageIq component. |
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.
ManageIq component
ManageIQ
import addReact from 'helpers'; | ||
import { YourComponent } from 'your-components/custom-component'; | ||
|
||
addReact('YourComponentName', YourComponent); |
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 like we may need to add something like...
If your component is not a pure function but a react class, you will need to call addReact('YourComponentName', (props) => <YourComponent {...props}>);
instead.
(But, maybe it would be better to fix that, let's talk tomorrow about how :))
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.
Yes, I hit that today.
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.
ui/register_react_component.md
Outdated
* props - object with props that will be mapped to your components (used when creating blueprint) | ||
* element - DOM element where this new instance of component should be rendered. | ||
```JS | ||
import componentFactory from 'helpers'; |
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.
componentFactory
is not a default export, should be import { componentFactory } from ...
ui/register_react_component.md
Outdated
import componentFactory from 'helpers'; | ||
|
||
const props = {}; | ||
componentFactory('YourComponentName', props, 'selector'); |
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.
Order of parameters is wrong, should be componentFactory('YourComponentName', 'selector', props)
. Also props is not mandatory and you can pass them via. data atribute of them DOM element
@karelhala The "New Instance" section doesn't actually show anything about using the component in manageiq. This needs an example of a rails view using a react component with params :). |
Also added a rails helper - ManageIQ/manageiq-ui-classic#3943 the short version: = react 'Whatever', {:pro => 'ps'} is the equivalent of - id = unique_html_id('react')
%div{:id => id}
:javascript
ManageIQ.component.componentFactory('Foo', '##{id}', '{"pro": "ps"}'); |
Oh, crap, I wanted to merge ManageIQ/manageiq-ui-classic#3943 ;-) Please, fix the documentation in a follow-up PR and don't tell anyone! |
Shared component API guide to show how to create and use and simple list of interaction with such component.