-
Notifications
You must be signed in to change notification settings - Fork 0
MAB-578 Display only grid in surveyjs #119
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
base: unesco-mab
Are you sure you want to change the base?
Conversation
src/utils/form/extractFields.ts
Outdated
| if (element.type === 'panel') { | ||
| await extractFields(element, fields, core); | ||
| } else { | ||
| if (element.type === 'resources' && element.displayOnly) { |
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.
Use guard clauses to handle special cases first
src/utils/form/extractFields.ts
Outdated
| await extractFields(element, fields, core); | ||
| } else { | ||
| if (element.type === 'resources' && element.displayOnly) { | ||
| // Don't store as field if question is display only |
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 comments
marianorelief
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.
left some comments
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.
- Still Missing TypeScript types
- All parameters are implicit 'any'
- Potential null reference errors
- Line 73: element.items.map() - no check if items exists
- Line 84: element.rows.map() - no check if rows exists
- Line 90: element.columns.map() - no check if columns exists
- Line 144: element.choices.map() - no check if choices exists
- Line 188: element.choices.map() - no check if choices exists
- Use element.items?.map() ?? [] or validate before mapping
- Still Missing displayOnly validation
- DRY
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.
@marianorelief I agree with your comment, thus it's a bit out of scope as the method was already there before Ronny worked on it, that's why he didn't do typing I think
I'll keep this PR active but won't merge it now because it should be good to do a bit more testing to prevent unexpected issue to raise due to type validation
I'll create another PR, with a simplified version of the code to go faster as we have tight deadline incoming
Description
Backend support for display-only resources grids in SurveyJS forms. This feature allows forms to display previous form data without storing the display-only questions as database fields.
The fix prevents validation errors when saving forms with display-only resources questions that don't have a relatedName property, as these questions are meant only for display purposes and should not be persisted.
Useful links
Type of change
How Has This Been Tested?
Test A: Add display-only resources question
Test B: Display-only grid rendering
Test C: Form submission
Screenshots
N/A
Checklist:
( * == Mandatory )
More explanation
https://www.loom.com/share/05a716d61b9744faaf51fb304c21d1e5?sid=f87cf896-582a-4f76-93ae-8ceed801b145