-
Notifications
You must be signed in to change notification settings - Fork 91
Add mapper generator #494
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 mapper generator #494
Conversation
Codecov Report
@@ Coverage Diff @@
## master #494 +/- ##
=======================================
Coverage 89.78% 89.78%
=======================================
Files 144 144
Lines 2437 2437
Branches 813 813
=======================================
Hits 2188 2188
Misses 249 249 Continue to review full report at Codecov.
|
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 good. I think we should finish these points:
- remove any PF(ar any other component library) references
- Instead of returning strings with component names, generate simple HTML inputs connected to the form API. Sometimes it might not be clear to how to use the
onChange
function - Include common packages into examples. We would not want to implement them again in each mapper
@@ -87,6 +87,16 @@ All packages are releasing together and they share the version number. | |||
|
|||
If your changes influence API or add new features, you should describe these new options in the `demo` repository. Thanks! | |||
|
|||
# Generating a mapper |
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 would the template word. it might seem that we are able to just generate fully featured component mapper.
scripts/generate-mapper.js
Outdated
type: 'input', | ||
message: 'Project name (i.e.: pf4, pf3, mui, blueprint, etc.):', | ||
validate(input) { | ||
if (/^([A-Za-z\d])+$/.test(input)) { |
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 allow a bit more characters. -, _
would be a good addition
scripts/generate-mapper.js
Outdated
1. Update styles in "packages/${componentmapper}-component-mapper/demo/index.html" | ||
2. Add dependencies in "packages/${componentmapper}-component-mapper/package.json", | ||
3. Mark the dependencies as globals/external in "packages/${componentmapper}-component-mapper/rollup.config.js" | ||
4. Mark the dependencies as externals in "packages/${componentmapper}-component-mapper/config/webpack.config.js" |
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.
This step is not necessary. Webpack is only used for dev server.
templates/component-mapper/README.md
Outdated
@@ -0,0 +1,179 @@ | |||
[](https://badge.fury.io/js/%40data-driven-forms%2Fpf4-component-mapper) |
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.
PF4 reference
@@ -0,0 +1,1949 @@ | |||
/* eslint-disable camelcase */ | |||
const input = { |
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 don't think it's necessary to copy this schema. I think we can even remove it completely
import React from 'react'; | ||
import PropTypes from 'prop-types'; | ||
|
||
const FieldArray = () => 'FieldArray'; |
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.
Same here I would show of the common package.
children: PropTypes.oneOfType([PropTypes.arrayOf(PropTypes.node), PropTypes.node]) | ||
}; | ||
|
||
export const Description = ({ children }) => <span>{children}</span>; |
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.
<p></p>
span is inline
children: PropTypes.oneOfType([PropTypes.arrayOf(PropTypes.node), PropTypes.node]) | ||
}; | ||
|
||
export const Form = ({ children, ...props }) => <form {...props}>{children}</form>; |
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.
noValidate
is missing. The form will prioritize HTML validation over ours.
import React from 'react'; | ||
import PropTypes from 'prop-types'; | ||
|
||
const Radio = () => 'Radio'; |
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.
Again we can use some basic implementation to showcase what props and how to use them in order to get this working. Mainly the onChange is important
import React from 'react'; | ||
import PropTypes from 'prop-types'; | ||
|
||
const Select = () => 'Select'; |
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.
^^
2179e12
to
00b9861
Compare
@Hyperkid123 changed, it creates a fully functional mapper now. All components should work. |
return ( | ||
<React.Fragment> | ||
<label htmlFor={name}>{label}</label> | ||
<input {...input} type="checkbox" disabled={isDisabled}/> |
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.
you are missing id={name}
for the label to work
🎉 This PR is included in version 2.4.0 🎉 The release is available on Demo can be found here! |
When user runs
It will ask him for a mapper name and it will automatically create the project folder.