xkjyeah / vue-google-maps

Google maps component for vue with 2-way data binding
https://xkjyeah.github.io/vue-google-maps/
1.88k stars 475 forks source link

Added ability to customize input using input named scoped slot #659

Closed TrendyTim closed 5 years ago

TrendyTim commented 5 years ago

I found a way to make it very innocuous by making the original input outside the slot (and the wrapping span) and conditionally rendering the original input and the span depending on if the slot is specified.

So the following get the same render output as before the change

 <gmap-autocomplete  @place_changed="processLocationChanged" 
         placeholder="Location Of Event" class="introInput">
        </gmap-autocomplete>

But this gets you a vuetify text field, wrapped in an unfortunately unavoidable (to the best of my knowledge) span.

<!-- if using a component other than vuetify, specify childRefName="refNameOfHTMLInput" in gmap-autocomplete  -->
 <gmap-autocomplete  @place_changed="processLocationChanged"  
          placeholder="Location Of Event" class="introInput">
                    <!-- Be sure to use v-slot:input="" the slotProps can be
                     whatever you want but you must use that name in place of 
                    slotProps elsewhere in the slot -->
                    <template v-slot:input="slotProps">

                       <!-- ref="input" is requried
                       v-on:listeners="slotProps.listeners" is required, rename slotProps here if you did in v-slot:input="slotProps" above
                       v-on:attrs ="slotProps.attrs" may be required, rename slotProps here if you did in v-slot:input="slotProps" above 
                       -->
                        <v-text-field outlined prepend-inner-icon="place" 
                             placeholder="Location" 
                             ref="input"
                            v-on:listeners="slotProps.listeners"
                            v-on:attrs="slotProps.attrs">
                         </v-text-field>
                    </template>
        </gmap-autocomplete>

The only requirements are that v-on:listeners and ref be set, i don't know if attrs really needs to be set but im making it available in the scope anyway.

In this case vuetify puts a ref="input" on its internal text input, and the JS implementation will look for a $refs.input which firstly finds the vueitfy component, so if there is a .$refs on that it will look further and use the childRefName (eg slot.$refs.input.$refs[childRefName] (which defaults to input) to get the reference to the input element.

diegoazh commented 5 years ago

@TrendyTim can you provide an example without vuetify and with other popular framework?

TrendyTim commented 5 years ago

I setup a little terst app so i could try with buefy and spend 30mins trying for figure out why it didn't work, i forgot to add the ref="input"

Here are two examples, one with a plain html input one with buefy (which just so happens to have the same internal ref name as vuetify

Plan Old Textbox

 <gmap-autocomplete  @place_changed="processLocationChanged" >
                    <template v-slot:input="slotProps">
                      <input ref="input" type="text" 
                             placeholder="slotted plain old textbox" 
                             v-on:listeners="slotProps.listeners" 
                              v-on:attrs="slotProps.attrs" />
                    </template>
        </gmap-autocomplete>

Buefy input

    <gmap-autocomplete  @place_changed="processLocationChanged" >
                    <template v-slot:input="slotProps">
                            <b-input ref="input"  
                                   placeholder="slotted buefy textbox"    
                                   v-on:listeners="slotProps.listeners" 
                                   v-on:attrs="slotProps.attrs"  ></b-input>
                    </template>
        </gmap-autocomplete>                 

Any other frameworks you would like an example with ?

BTW: I did introduce a dependency on Vue 2.6 due to the slot usage though so not sure if you want to take that on yet or not but i don't think its too bad.

I take it that the test failure " Evaluation failed: TypeError: Cannot read property 'lat' of undefined" had nothing to do with what i added, i could not see how that could be related.

diegoazh commented 5 years ago

@TrendyTim thank you for the examples, my intention was understand this change, I try to avoid add some code that is specific for vuetify, on the other hand, this test fails on others PR's too, I try understand why it fails and improve the tests suite. On other cases I requested that you run the tests suite locally and post a print of the result, if you can, please provide that print result. If you have knowledge about lab and you can fix the error on the test please feel free to create a new PR in order to fix that. Thank you for your contribution.

TrendyTim commented 5 years ago

@diegoazh You are very welcome, im happy to help where i can, i totally understand where you are coming from, i agree adding features that are specifically for one framework opens a pandoras box, which is why i made extra effort to make it work with (theoretically) any framework.

I'm glad its not just my PR then.

I only very recently started using npm/webpack/vuecli etc i actually have no idea what lab is yet, i really should look into tests before i get too much code written.

If i run "npm run test" locally on my fork it runs all tests fine, just some complaints from the linter where the code i added in autocomplete had an indent and multi space issue which i can fix up if you like.

Is the error perhaps specific to travis-ci environment (im on windows 10, node 10.1 ) ?

TrendyTim commented 5 years ago

i made the linter fixes in my fork, but not sure if i should close this PR and create a new one or what ... this is my first PR ever so im very much a n00b, if you want me to include those changes let me know if you have a preference :)

TrendyTim commented 5 years ago

Oops looks like i accidently inlcuded the readme in the PR, doesnt quite make sense of the main package site now @diegoazh Do you want to fix that up or should i write up the doc further down the file about how to use it, and PR it

TheBestMoshe commented 5 years ago

Which version will this feature be released? This is exactly what I was looking for.

This would save me lot's of manual work since I was having issues with vuetify-google-autocomplete in some browsers.

TrendyTim commented 5 years ago

@TheBestMoshe

I don't know what the release schedule is but in order to use it in my deploy pipeline i built my fork and uploaded it to npmjs as a stopgap measure, i don't intend to keep it updated but it is the current latest version from this git with my mod applied (so you can just import that one instead of this one and then switch back when an official build is pushed ... keep in mind my npmjs build is based on 0.12.1 where the current official npmjs build is based on 0.10.8)

https://www.npmjs.com/package/vue2-google-maps-withscopedautocomp

nfunwigabga commented 4 years ago

great PR. I installed your fork and it is exactly what I needed... thank you!

TrendyTim commented 4 years ago

You're welcome, im glad others find it useful, i'm surprised at how easy it was (there is probably a better way but its beyond my current skills)

Hopefully they will publish the newer version to the official npm repo soon.

diegoazh commented 4 years ago

@xkjyeah when you think you can land a new release? the tests are fixed and build pass now, I hope you can before christmas, thank you ๐Ÿ‘๐Ÿ‘๐Ÿ‘๐Ÿ‘

davydnorris commented 4 years ago

@TrendyTim is there any special trick to this? I am using Vuetify and have literally copied your example verbatim into my project but it's as if $scopedSlots is not working properly and the v-if fails - it just gives me the default input field instead of what's in my template

TrendyTim commented 4 years ago

@davydnorris Are you using the official or my forked build ?

There is no special trick it should just work if your using my build, as to the best of my knowledge this PR hasn't been built to npm yet.

davydnorris commented 4 years ago

@TrendyTim that may explain it! Just looked and yep it's not in the npm package :-(

Thanks for the speedy reply!

TrendyTim commented 4 years ago

@davydnorris no worries, if you want to use my forked npm package it's listed above in

https://github.com/xkjyeah/vue-google-maps/pull/659#issuecomment-548600462

zealotrahl commented 4 years ago

Trying to add autocomplete with element.io el-input, and getting no changes, @TrendyTim Could you look? https://element.eleme.io/#/en-US/component/input

TrendyTim commented 4 years ago

@zealotrahl Which npm package are you using, the official one or my temporary one ?

If your getting no changes, I suspect the either your not using the custom build (still waiting on @xkjyeah to deploy a new build so my temp one or doing it yourself is the only way), or your slot isn't right is my guess.

nfunwigabga commented 4 years ago

Can you share your code? Do you have the ref attribute on the input?

davydnorris commented 4 years ago

@zealotrahl - Tim is right, the published npm package does not have his changes yet. That was my problem too

zealotrahl commented 4 years ago

@diegoazh Thanks, yes I installed from npm. It's funny, because autocomplete example is listed at the main README, of the master :).

TrendyTim commented 4 years ago

@zealotrahl I made a PR to note that the offical npmjs doesnt support it with a link to mine ... until @xkjyeah or it seems that @diegoazh might have NPM permissions to push as well, so that would be if someone could push a new one.

diegoazh commented 4 years ago

How the owner says in another thread, he is very busy at this moment and he can't review the code and land a new release, he suggested fork the project and land it like a different package, in order to do that a plan to land an updated version of this package this weekend, if you like this and you want to help the development of this project I will happy to work with you on the development and maintenance of this libraries.

diegoazh commented 4 years ago

@zealotrahl I made a PR to note that the offical npmjs doesnt support it with a link to mine ... until @xkjyeah or it seems that @diegoazh might have NPM permissions to push as well, so that would be if someone could push a new one.

I published all new features added in the lasts months to gmap-vue in npm the fork is in https://github.com/diegoazh/gmap-vue if you want to collaborate or send a pull request I will be happy to review any incoming code.