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

v-if / v-else not working with :type + v-model #6918

Closed
FranckFreiburger opened this issue Oct 25, 2017 · 14 comments · Fixed by #6955
Closed

v-if / v-else not working with :type + v-model #6918

FranckFreiburger opened this issue Oct 25, 2017 · 14 comments · Fixed by #6955
Labels

Comments

@FranckFreiburger
Copy link

Version

2.5.2

Reproduction link

https://jsfiddle.net/50wL7mdz/71931/

Steps to reproduce

run the jsfiddle example

What is expected?

v-else to work

What is actually happening?

v-else seems to have no effect


maybe related/duplicate with issue #6917 or issue #6907

@yyx990803 yyx990803 added the bug label Oct 25, 2017
@misoguy
Copy link
Contributor

misoguy commented Oct 26, 2017

I'd like a take a look at this if no one is on it yet :)

@FranckFreiburger
Copy link
Author

This seems to be strongly related to the dynamic type :type="'text'"

@gebilaoxiong
Copy link
Member

It seems that we need to add a pre condition

in https://github.com/vuejs/vue/blob/dev/src/platforms/web/compiler/modules/model.js#L31

misoguy added a commit to misoguy/vue that referenced this issue Oct 26, 2017
@gebilaoxiong
Copy link
Member

@misoguy OK, you come first 😭

@gebilaoxiong
Copy link
Member

gebilaoxiong commented Oct 29, 2017

We also need to consider v-else-if ...

I'm sorry to dirty your issue 😃

@misoguy
Copy link
Contributor

misoguy commented Oct 30, 2017

I've been digging into this issue for a while and realized that it's not that simple.
I have a roughly implemented working code for
<div v-if="ok">haha</div><input v-else :type="type" v-model="test">
but I think I need to write test for

<div v-if="ok">haha</div>
<input v-else-if="test==='b'" :type="type" v-model="test">
<input v-else :type="type" v-model="test">

and make this case to work as well. I'll work on this and send a PR :)

@misoguy
Copy link
Contributor

misoguy commented Oct 30, 2017

@gebilaoxiong My browser didn't refresh and didn't see your comment and PR before commenting :( I guess this issue is closed then?

@gebilaoxiong
Copy link
Member

gebilaoxiong commented Oct 30, 2017

@misoguy

I thought no one was in the process of it, so it is repaired

I don't know if there is any other problem

@misoguy
Copy link
Contributor

misoguy commented Oct 30, 2017

@gebilaoxiong I should have commented that I was gonna work on it during the weekends.
My bad :( I tried out your code and tested it with

<div v-if="foo">text</div>
<input v-else-if="bar" :type="type" v-model="test">
<input v-else :type="type" v-model="test">

as well. I don't think there is any more problem regarding this issue 👍

@gebilaoxiong
Copy link
Member

gebilaoxiong commented Oct 30, 2017

@misoguy

you can use your code to open a PR

I will merge my code to yours

@misoguy
Copy link
Contributor

misoguy commented Oct 30, 2017

@gebilaoxiong Would that be ok? But your implementation and test code seems much better than mine! :) I'll open a PR as you suggested and let you decide whether or not to merge yours with mine :)

@misoguy
Copy link
Contributor

misoguy commented Oct 30, 2017

@gebilaoxiong I have opened a PR as you suggested and only included the code to check v-else since I didn't want to get the credit for your work regarding v-else-if :)
It would be nice to contribute for the first time on vue.js!

By the way, while implementing my first PR on vue.js, I found that it was wonderfully maintained with all the test codes that were written! Great library! 👍

@gebilaoxiong
Copy link
Member

gebilaoxiong commented Oct 30, 2017

@misoguy have a nice day. 😄

@misoguy
Copy link
Contributor

misoguy commented Oct 30, 2017

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants