Skip to content

Conversation

@YamenSharaf
Copy link
Contributor

@YamenSharaf YamenSharaf commented Sep 12, 2018

Please make sure these boxes are checked before submitting your PR, thank you!

  • Make sure you follow Element's contributing guide (中文 | English | Español).
  • Make sure you are merging your commits to dev branch.
  • Add some descriptions and refer relative issues for you PR.

@element-bot
Copy link
Member

element-bot commented Sep 12, 2018

Deploy preview for element ready!

Built with commit 77563d4

https://deploy-preview-12733--element.netlify.com

@YamenSharaf
Copy link
Contributor Author

This is to fix this issue I posted here earlier

#12732

@YamenSharaf
Copy link
Contributor Author

@ziyoung This is the payload emitted by the Form component at the moment. It does not emit the current error message.

screen shot 2018-09-12 at 10 04 14 pm

Personally, I think both Form and FormItem should emit that error message to enable developers to wrap these components in any way they want.

I disagree with how the payload is being evaluated right now on the Form (as an array). An object of declared keys would be the way to go usually, but for the sake of backwards compatibility, I think It should be added as the third entry in that array anyway.

@abdallanayer
Copy link
Contributor

@ziyoung I agree with @YamenSharaf that we need more flexibility with the error messages to give the developers more power to control how they show the errors

@ibufu
Copy link
Contributor

ibufu commented Sep 21, 2018

@YamenSharaf Is it better to put the error message in the validate event of form?

@ziyoung
Copy link
Contributor

ziyoung commented Sep 21, 2018

If validate event emits error message, is it necessary to add error event?

@YamenSharaf
Copy link
Contributor Author

@ibufu @ziyoung Personally I think we should have both, because:

  • The bigger use case for this is listening to a specific error on a specific form-item. Probably to take action on such error.

  • Each form item could have multiple errors associated with it at the time (Although it's showing one at a time). having a list of errors would be useful to trigger an action on specific error

  • The current validate event on form should only care about the errors visible to the end-user, and the way its payload is structured is not ideal (but it makes sense for it to emit the error itself anyway).

@YamenSharaf
Copy link
Contributor Author

@ziyoung Also made this PR for having form emit the actual error on validate events. This doesn't seem very wieldy since ElForm emits that event all the time for each individual form element. I still think it can be useful though. #12860

@wacky6
Copy link
Contributor

wacky6 commented Sep 24, 2018

errors are actually a list of new Error(message). These messages are meant to be read by user (not to be used in program logic).

What would you plan to do with these strings? For example?

if (errors.find(e => e.message.includes('Field A is required'))) {
    blame_user_with_dialog(...)
}

String comparison (on error message) does not make sense to me.

@YamenSharaf
Copy link
Contributor Author

@wacky6 String comparison is icky, but sometimes you wanna respond to a certain error in a certain way, let that be dimming a button or popping an alert. It could be that I want errors to appear on the side of the form as opposed underneath inputs.

The way things are set up now there's no way to access these errors for any reason which I don't think is very flexible.

@wacky6
Copy link
Contributor

wacky6 commented Sep 24, 2018

In that case, I would append additional codes after calling validate(), and check the v-model / form's value directly.

Validators are supposed to show a message, but nothing more.

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.

6 participants