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

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

Merged
merged 2 commits into from
Dec 20, 2023

Conversation

EtienneDOYON
Copy link
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

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,
},

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;
}

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
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
2 checks passed
@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