-
Notifications
You must be signed in to change notification settings - Fork 375
feat(FormContext): Add context for Form to manage input state #8716
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
| "scripts": { | ||
| "build:umd": "rollup -c --environment IS_PRODUCTION", | ||
| "clean": "rimraf dist", | ||
| "clean": "rimraf dist components layouts helpers next deprecated node_modules", |
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 does not pertain to this exact change, but something noticed when running clean, I believe these built sub-directories should be included.
| "analyze": "yarn build:docs --analyze", | ||
| "build:docs": "pf-docs-framework build all", | ||
| "clean": "rimraf .cache public static/assets static/base.css src/generated/**/*.js", | ||
| "clean": "rimraf .cache public static/assets static/base.css src/generated/**/*.js node_modules", |
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.
When using a local version of the documentation-framework, it helps to clear out these node_modules, as sometimes not doing so leads to false-positives/negatives.
| 'FormFieldGroupHeader', | ||
| 'FormFieldGroupHeaderTitleTextObject', | ||
| 'FormContextProps', | ||
| 'FormContextProviderProps', |
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.
Both of these types should probably be marked as beta, but there is no way to do so yet with the PF documentation-framework. The only use-case is an example called FormState, which is marked as beta, so hopefully that will suffice for now.
|
Preview: https://patternfly-react-pr-8716.surge.sh A11y report: https://patternfly-react-pr-8716-a11y.surge.sh |
bb7f81c to
0b95e06
Compare
wise-king-sullyman
left a comment
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.
Neat! I just have a couple small nit things and one test I'm not sure about.
packages/react-core/src/components/Form/__tests__/FormContext.test.tsx
Outdated
Show resolved
Hide resolved
packages/react-core/src/components/Form/__tests__/FormContext.test.tsx
Outdated
Show resolved
Hide resolved
7b7e044 to
c606f44
Compare
kmcfaul
left a comment
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.
lgtm just needs a rebase
c606f44 to
5afe1fb
Compare
|
Github wont let me merge this because it says there are still conflicts needing to be resolved? |
c923234 to
a0aa692
Compare
Yeah, looks like another PR merged today led to another conflict. I rebased and resolved that conflict just now. |
What: Closes #7507