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

Lmb 1226 | can mark custom field required #458

Merged

Conversation

JonasVanHoof
Copy link
Contributor

@JonasVanHoof JonasVanHoof commented Jan 13, 2025

Description

You should be able to add a telephone number field that has phone validation + make it possible to mark a field as required

How to test

Add these fields to a form and see if the validations are correct.

Links to other PR's

Attachments

image

@JonasVanHoof JonasVanHoof self-assigned this Jan 13, 2025
@JonasVanHoof JonasVanHoof changed the base branch from master to lmb-182-can-modify-custom-field January 13, 2025 09:04
@JonasVanHoof JonasVanHoof force-pushed the lmb-181-add-more-displaytypes branch from 5806cfb to 092a696 Compare January 14, 2025 12:29
@JonasVanHoof JonasVanHoof marked this pull request as ready for review January 14, 2025 16:10
@JonasVanHoof JonasVanHoof force-pushed the lmb-181-add-more-displaytypes branch from 0b99724 to 53ae14b Compare January 15, 2025 10:46
@JonasVanHoof JonasVanHoof changed the title Lmb 181 | Add more display types Lmb 1226 | can mark custom field required Jan 15, 2025
Base automatically changed from lmb-182-can-modify-custom-field to karel/lmb-333-create-form-extensions January 15, 2025 15:36
Comment on lines 260 to 262
get canShowRequiredToggle() {
return this.displayType.uri !== ADRES_CUSTOM_DISPLAY_TYPE;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Important the adress component cannot handle the validations as other rdf inputs do

Copy link
Collaborator

Choose a reason for hiding this comment

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

should we not just make it handle that then? Its value is just a uri so i think the default checks should work for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It works with an adres as displaytype but I can't seem to make it work for the library entry "Verblijfplaats", the path of the validation and the field are the same. I removed the restriction of the required toggle not showing, the remark will than be that the verblijfplaats field will CAN be filled in as empty.

@Rahien Rahien force-pushed the karel/lmb-333-create-form-extensions branch from 14ba654 to 439b213 Compare January 20, 2025 08:33
@JonasVanHoof JonasVanHoof force-pushed the lmb-181-add-more-displaytypes branch 2 times, most recently from 599008b to 55e0bc7 Compare January 22, 2025 09:31
Copy link
Collaborator

@Rahien Rahien left a comment

Choose a reason for hiding this comment

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

apart from this, when changing field type a couple of times, the "dit veld is verplicht" error message is duplicated and see if you can fix the behavior when something that is not a number or date is filled in but the type is changed to number or date. Nothing is shown and the form can be saved because there is a value. Maybe it should clear that value in that case?

Comment on lines 260 to 262
get canShowRequiredToggle() {
return this.displayType.uri !== ADRES_CUSTOM_DISPLAY_TYPE;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we not just make it handle that then? Its value is just a uri so i think the default checks should work for it?

@JonasVanHoof JonasVanHoof force-pushed the lmb-181-add-more-displaytypes branch from 6043bb7 to 95c46fb Compare January 23, 2025 09:17
@JonasVanHoof JonasVanHoof merged commit 01f4d46 into karel/lmb-333-create-form-extensions Jan 24, 2025
2 checks passed
@JonasVanHoof JonasVanHoof deleted the lmb-181-add-more-displaytypes branch January 24, 2025 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants