-
Notifications
You must be signed in to change notification settings - Fork 4k
FormsManager #3046
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
FormsManager #3046
Conversation
- FormsData is now an interface rather than a class - FormsManager updates FormsData via immer.produce - Form is now its own class, and has its own styled-components.ts
| export class Form extends PureComponent<Props> { | ||
| public render = (): ReactNode => { | ||
| return ( | ||
| <StyledForm data-testid="stForm" width={this.props.width}> | ||
| {this.props.children} | ||
| </StyledForm> | ||
| ) | ||
| } | ||
| } |
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.
Form is going to get some new functionality in a future PR, and it was convenient to make it a class in this PR to support that upcoming work.
| // sets, maps, and arrays. | ||
| formsMgr.setPendingForms(new Set(["one", "two"])) | ||
| expect(() => formsData.formsWithPendingChanges.clear()).toThrowError( | ||
| "[Immer] This object has been frozen and should not be mutated" |
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 relies heavily on the implementation of Immer. Meaning, if Immer decides to change the error message tomorrow - this test would fail for "no reason".
Can we test the expected behavior using Object.isFrozen() or some other way?
kantuni
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.
Looks great!
* st_form: Added "allow-downloads" to the sandbox attributes (streamlit#3053) Small fix for `make pylint` command (streamlit#3062) Set genericColors properly and make theme defs more consistent (streamlit#3051) Downgrade st.warning in local_sources_watcher to a log (streamlit#3050) Extend docstring of st.image (streamlit#3055) Move docs.streamlit.io to Segment (streamlit#3005) Fix bokeh example in docs (`legend` arg) (streamlit#2907) Remove incorrect markdown table styling (streamlit#3038) document getattr Show the beta_warning only a single time per object object_beta_warning: handle every __ magic method object_beta_warning: handle the subscript operator beta_util_test Move secrets out of beta [Security] Upgrade y18n to 4.0.1 or later (streamlit#3041) Fix gap not working on Safari (streamlit#3042) formatting call _repr_html_ when available
# By Tim Conkling (7) and others # Via GitHub (3) and Tim Conkling (1) * st_form: FormsManager (streamlit#3046) Added "allow-downloads" to the sandbox attributes (streamlit#3053) Small fix for `make pylint` command (streamlit#3062) Set genericColors properly and make theme defs more consistent (streamlit#3051) Downgrade st.warning in local_sources_watcher to a log (streamlit#3050) Extend docstring of st.image (streamlit#3055) Move docs.streamlit.io to Segment (streamlit#3005) Fix bokeh example in docs (`legend` arg) (streamlit#2907) Remove incorrect markdown table styling (streamlit#3038) document getattr Show the beta_warning only a single time per object object_beta_warning: handle every __ magic method object_beta_warning: handle the subscript operator beta_util_test Move secrets out of beta [Security] Upgrade y18n to 4.0.1 or later (streamlit#3041) Fix gap not working on Safari (streamlit#3042) formatting call _repr_html_ when available # Conflicts: # frontend/src/components/core/Block/Block.tsx # frontend/src/components/widgets/Form/Form.tsx # frontend/src/components/widgets/Form/FormSubmitButton.tsx # frontend/src/components/widgets/Form/FormsData.test.ts # frontend/src/components/widgets/Form/FormsManager.ts
An
st_formrefactor that introduces the ImmerJS library, and lays the groundwork for the upcoming "warn on missing submit button" PR