-
Notifications
You must be signed in to change notification settings - Fork 7
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
fix: lexemes cannot be disabled as button is rendered outside of form #776
Conversation
Deployment previews on netlify for branch
|
|
||
<v-card-actions> | ||
<v-spacer></v-spacer> | ||
<v-tooltip top> | ||
<template v-slot:activator="{ on, attrs }"> | ||
<v-btn color="primary" v-bind="attrs" v-on="on" text type="submit"> | ||
<v-btn v-bind="attrs" v-on="on" color="primary" type="submit" form="formLexeme"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First problem here would be that albeit the button is within the parent form in the template, Vue will hoist it outside of the form when rendering the modal, meaning submitting it would go nowhere as there's no associated form.
There are two solutions to the problem:
- give the form an id and reference it here (like I did)
- on clicking this button, programatiicaly submit the form
I think both is ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks nice
valid: false, | ||
dialog: false, | ||
userConfirmationString: 'wbstack-important-change', | ||
rules: { | ||
match: (value) => | ||
value === this.userConfirmationString || 'Please enter the confirmation' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Second problem was that this
here would be the Vue component, not the data
object, thus returning undefined
instead of the value of userConfirmationString
Also the comparison is borked and would return either true|'Please enter the confirmation'
instead of true|false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm very unclear how this ever really worked
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm; also checked it worked as expected in the prod version of netlify
Ticket https://phabricator.wikimedia.org/T353533