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

Provide target parameter field names as message placeholders #2290

Closed
rklfss opened this issue Sep 4, 2019 · 13 comments · Fixed by #2363
Closed

Provide target parameter field names as message placeholders #2290

rklfss opened this issue Sep 4, 2019 · 13 comments · Fixed by #2363
Labels
✨ enhancement a "nice to have" feature

Comments

@rklfss
Copy link

rklfss commented Sep 4, 2019

Consider the "confirmed" rule.
The german message "Die Bestätigung von {_field_} stimmt nicht überein" literally means "The confirmation OF {_field_} does not match." which is odd, because it is the confirmation of the target field, not the field itself.

My proposal is to provide additional "internal" message placeholders that contain the name of the target parameter fields.

So as en example for the "confirmed" rule you have the parameter "target" which is the value of the target field.
Just add "{_target_}" to the parameter list and provide the name of the target field here.

Technically its "{_<targetParameterName>_}" because you could have multiple target parameters.

This would lead to a perfect message "Die Bestätigung von {_target_} stimmt nicht überein."/"The confirmation of {_target_} does not match." which is what the confirmed rule really is about to say.

Thanks.

@logaretm logaretm added the ✨ enhancement a "nice to have" feature label Sep 4, 2019
@logaretm
Copy link
Owner

logaretm commented Sep 4, 2019

Sounds good to me, I will try to implement it for the next release.

@rklfss
Copy link
Author

rklfss commented Sep 4, 2019

Thanks. Current workaround for me is to use the same name like the target field but the desired label on my input. But I think that's not how it is meant to be.

@davestewart
Copy link
Collaborator

davestewart commented Sep 21, 2019

Any reason the original message doesn't just use English @makana !?

By the way, I also need this functionality.

@davestewart
Copy link
Collaborator

@logaretm - would you accept a PR for this if you don't have time?

I have a post-process function I run to do the same thing, but would be much nicer built-in of course.

@logaretm
Copy link
Owner

@davestewart If it is not a problem that would be great, I didn't get a chance to implement this yet.

@davestewart
Copy link
Collaborator

I'm guessing it's not too complex in the validator src, but you would want tests and docs update for a full PR, right? Anything else?

@logaretm
Copy link
Owner

That's more than enough, thanks!

@davestewart
Copy link
Collaborator

davestewart commented Sep 22, 2019

OK... got a failing test!

Code looking OK? In the right place?

image

@logaretm
Copy link
Owner

Yep, looking good.

@logaretm
Copy link
Owner

Something like {target} or your second suggestion. Since we shouldn't localize this.

@davestewart
Copy link
Collaborator

Sorry - I deleted the question, as I realised we can just use the field key, so password not Password.

@rklfss
Copy link
Author

rklfss commented Sep 23, 2019

Thanks for implementing this!

@davestewart
Copy link
Collaborator

Usage for single and multiple targets here:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ enhancement a "nice to have" feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants