Skip to content

feat(meta): add option to display mandatory field hint#98

Merged
EtienneDOYON merged 2 commits intomasterfrom
add-meta-display-required-hint
Dec 20, 2023
Merged

feat(meta): add option to display mandatory field hint#98
EtienneDOYON merged 2 commits intomasterfrom
add-meta-display-required-hint

Conversation

@EtienneDOYON
Copy link
Copy Markdown
Contributor

Description

Add the possibility for a given form to handle metas.
Add the meta shouldDisplayRequiredHint that conditions displaying a * next to a field's label, if that label is mandatory.

Related Issue

None.

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • All new and existing tests passed.

@EtienneDOYON EtienneDOYON added 🧑‍⚖️ Tech review needed Pull Request is ready for review, let's go ! Bedrock When the author is a member of the organization 🫀 Core Package updates labels Dec 13, 2023
@EtienneDOYON EtienneDOYON self-assigned this Dec 13, 2023
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Dec 14, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (740e3b1) 74.48% compared to head (42609a0) 74.68%.
Report is 2 commits behind head on master.

❗ Current head 42609a0 differs from pull request most recent head 199ebe1. Consider uploading reports for the commit 199ebe1 to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #98      +/-   ##
==========================================
+ Coverage   74.48%   74.68%   +0.19%     
==========================================
  Files          28       28              
  Lines         388      391       +3     
  Branches      123      125       +2     
==========================================
+ Hits          289      292       +3     
  Misses         64       64              
  Partials       35       35              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines +64 to +66
formMeta: {
shouldDisplayRequiredHint: true,
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 suggestion: ‏should be the opposite to match maximum case like actual

Comment on lines +34 to +36
if (!validation || !validation?.required?.value) {
shouldDisplayRequiredHint = false;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 suggestion: ‏and the invert condition here. Should be false by default and set to true here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The goal here is to force the boolean to be false if and only if the field is optionnal. If it is required, we want to use the default value.

@EtienneDOYON EtienneDOYON force-pushed the add-meta-display-required-hint branch from 42609a0 to 199ebe1 Compare December 20, 2023 10:18
@EtienneDOYON EtienneDOYON merged commit e15dbf8 into master Dec 20, 2023
@EtienneDOYON EtienneDOYON deleted the add-meta-display-required-hint branch December 20, 2023 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bedrock When the author is a member of the organization 🫀 Core Package updates 🧑‍⚖️ Tech review needed Pull Request is ready for review, let's go !

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants