Skip to content
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

Higlight required fields #296

Merged
merged 3 commits into from
May 25, 2021
Merged

Conversation

saulipurhonen
Copy link
Contributor

Description

All required fields are not highlighted and this can confuse user.

Related issues

Closes #291

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Changes Made

  • Handle sections that are required but don't contain any required fields
  • Highlight required text areas
  • Highlight required One Of fields
  • Asterisk after FieldArray heading if section is required

Testing

  • Tests do not apply

@saulipurhonen saulipurhonen added bug Something isn't working enhancement New feature or request labels May 6, 2021
@saulipurhonen saulipurhonen added this to the Beta - User Interaction milestone May 6, 2021
@saulipurhonen saulipurhonen self-assigned this May 6, 2021
@saulipurhonen saulipurhonen marked this pull request as draft May 7, 2021 06:04
@saulipurhonen
Copy link
Contributor Author

Found some issues where non-required field were handled as required.
There's also some fields that need to be required but nesting loses the required attribute.

@saulipurhonen saulipurhonen force-pushed the bugfix/highlight-required-fields branch from 25e74b2 to 83db23b Compare May 7, 2021 13:05
@saulipurhonen saulipurhonen force-pushed the bugfix/highlight-required-fields branch from 83db23b to 2f64e88 Compare May 7, 2021 13:16
@saulipurhonen
Copy link
Contributor Author

Logic needs some fine tuning since highlighting of first field of deeply nested section (which ancestor is a required property) doesn't work.

In example schema, Design is required property and therefore it needs data to pass validation.

Example in deeply nested field that should be required:
image

In above image user has selected Pool as Sample Reference. In this case first field of child section should be required.

If user chooses Invidual Sample from options, highlighting works since first field gets required property.

Also, duplicate heading for Pool doesn't look right in design perspective. It works as should if checked against schema.

WIP 2


Fix for nested properties


WIP
@hannyle
Copy link
Contributor

hannyle commented May 21, 2021

@saulipurhonen I kinda fixed the issue with requiring and highlighting the first field of deeply nested object. I also fixed for some fields that are not required originally but still highlighted.

There are still some kinds of error with oneOf field (I guess) especially with Analysis object that even though I filled everything the form couldn't submit. We can investigate this more.

@hannyle hannyle self-assigned this May 21, 2021
@saulipurhonen saulipurhonen marked this pull request as ready for review May 24, 2021 11:58
Copy link
Contributor

@hannyle hannyle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the issue was actually traverseFields object argument in FormOneOfFied.
It's good that you noticed this.

@blankdots blankdots merged commit 4e0fc87 into develop May 25, 2021
@blankdots blankdots deleted the bugfix/highlight-required-fields branch May 25, 2021 14:48
@blankdots blankdots mentioned this pull request Aug 11, 2021
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants