-
Notifications
You must be signed in to change notification settings - Fork 2
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
Form autosave #144
Form autosave #144
Conversation
✅ Deploy Preview for i-vresse-workflow-builder ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #144 +/- ##
==========================================
- Coverage 66.24% 65.61% -0.63%
==========================================
Files 57 57
Lines 4076 4235 +159
Branches 338 344 +6
==========================================
+ Hits 2700 2779 +79
- Misses 1372 1452 +80
Partials 4 4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
2ec08c5
to
3a8df4a
Compare
…adaptations. tests: fix failing unit and integration tests
61a1b03
to
e64b922
Compare
related to #147 |
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.
Very nice ux improvement.
As a user I could not find big problems except the following
Given the initial page loaded of the haddock3-download app then adding the topoaa node by clicking it will mark both the global parameters and topoaa node as invalid in the middle column. I expected to only have global invalid. When I swich back to global and fix the error then the topoaa is still incorrectly shown as invalid.
The application in apps/* which are not haddock3-download are in auto save mode and also have a save button. Which is a bit strange, but not that bad that it needs fixing.
Finding an error in a big form like for guru catalog and emref node can be hard. It would be nice if in CollapsibleField component you could show that child field has an error. This could be done by adding
const hasError = Object.keys(props.errorSchema).length > 0 ? 'text-danger' : ''
...
<div className={'card-header ' + hasError}>
Could you add this to the CollapsibleField and make it look similar to other errors?
As a developer I have added some inline suggestions.
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.
Nice , all the apps have consistent autosave. Also linting gives no errors.
I have just a single minor inline suggestion for ts-standard version.
I made some changes to fix topoaa and rsmdmatrix + a workaround in #156
Feel free to review my changes and merge this PR.
Save changes in the form automatically. The autosave option is enabled by default.
UI changes:
form-control is-invalid
.Error notification in the workflow
Preferred approach instead of #138
Closes #138