Code Review Asked on December 21, 2021
Users input their pin-code which is checked against the database and returned with the locality, district and state associated with that pin-code. Multiple localities may be returned so the options are passed to a select element so the user can choose theirs.
Old values are also passed as props to the Vue component and displayed in event of server side validation error.
<template>
<div>
<div class="form-row">
<b-form-group class="col-6" label="Pincode" label-for="pincode">
<b-form-input
id="pincode"
name="pincode"
v-model="pincode"
placeholder="Pincode"
type="number"
min="100000"
max="999999"
:state="pincodeState"
></b-form-input>
<b-form-invalid-feedback id="pincode-live-feedback">
Pincode must be 6 digits
</b-form-invalid-feedback>
</b-form-group>
<b-form-group
class="col-6"
label="Locality (Please Select)"
label-for="locality"
>
<b-form-select
id="locality"
name="locality"
class="form-control"
v-model="locality"
:options="l_options"
:state="pinValid"
>
</b-form-select>
<b-form-invalid-feedback id="locality-live-feedback">
Pincode doesn't exist
</b-form-invalid-feedback>
</b-form-group>
</div>
<div class="form-row">
<b-form-group class="col-6" label="District" label-for="district">
<b-form-input
id="district"
type="text"
class="form-control"
v-model="district"
:state="pinValid"
disabled
></b-form-input>
<b-form-invalid-feedback id="district-live-feedback">
Pincode doesn't exist
</b-form-invalid-feedback>
</b-form-group>
<b-form-group class="col-6" label="State" label-for="state">
<b-form-input
id="state"
type="text"
class="form-control"
v-model="state"
:state="pinValid"
disabled
></b-form-input>
<b-form-invalid-feedback id="state-live-feedback">
Pincode doesn't exist
</b-form-invalid-feedback>
</b-form-group>
<input type="text" hidden name="district" :value="district" />
<input type="text" hidden name="state" :value="state" />
</div>
</div>
</template>
<script>
export default {
name: "AddressReceiver",
props: {
p_old: {
default: null,
},
l_old: {
default: null,
},
d_old: {
default: null,
},
s_old: {
default: null,
},
},
data() {
return {
pincode: this.p_old,
locality: this.l_old,
l_options: [],
district: this.d_old,
state: this.s_old,
pincodeState: null,
pinValid: null,
};
},
methods: {
pincodeSearch() {
axios
.get("/api/pincode/" + parseInt(this.pincode))
.then((response) => {
this.pinValid = true;
this.resetIn();
for (let i = 0; i < response.data.length; i++) {
this.l_options.push(response.data[i].locality);
}
if (this.l_old) {
this.locality = this.l_old;
} else {
this.locality = response.data[0].locality;
}
this.district = response.data[0].district;
this.state = response.data[0].state;
})
.catch((error) => {
this.pinValid = false;
this.resetIn();
});
},
resetIn() {
this.l_options = [];
this.locality = null;
this.district = null;
this.state = null;
},
},
mounted() {
if (this.p_old) {
this.pincode = this.p_old;
this.pincodeSearch();
}
},
watch: {
pincode(newPin) {
if (newPin >= 100000 && newPin <= 999999) {
this.pincodeSearch();
this.pincodeState = true;
} else {
this.resetIn();
this.pinValid = null;
this.pincodeState = false;
if (!newPin) {
this.pincodeState = null;
}
}
},
},
computed: {
pincodeValid() {},
},
};
</script>
<address-receiver p_old="{{ old('pincode') }}"
l_old="{{ old('locality') }}"
d_old="{{ old('district') }}"
s_old="{{ old('state') }}"
>
</address-receiver>
Is there a more elegant way of implementing the old data? I am doing the API call again and getting the options after which the old value is set as the selected value. Any criticism is welcome. I am new to Vue and would love to learn.
The function method pincodeSearch is called whenever the pincode value changes and is vald. It would likely be wise to consider minimizing server requests by debouncing the function and/or cancelling existing requests if the method is called rapidly in short succession.
It looks like you started to add a computed property pincodeValid but didn't implement and use it. It could be something as simple as this example:
pincodeValid: function () {
return this.pincode >= 100000 && this.pincode <= 999999
}
then instead of needing the watcher, a method could be added and bound to the @input attribute of the pincode input:
pincodeChange: function() {
if (this.pincodeValid) {
//this should probably be debounced to minimize server requests
this.pincodeSearch();
}
else {
this.resetIn();
this.pinValid = null
}
}
Then the whole watch section can be eliminated.
According to the VueJS documentation:
When you have some data that needs to change based on some other data, it is tempting to overuse
watch- especially if you are coming from an AngularJS background. However, it is often a better idea to use a computed property rather than an imperativewatchcallback. 1
While it may not help this situation much, it is better to get into the practice of using computed properties instead of watchers where possible, since "computed properties are cached based on their reactive dependencies. A computed property will only re-evaluate when some of its reactive dependencies have changed."2
There is this loop in the callback to the axios.get() call:
for (let i = 0; i < response.data.length; i++) { this.l_options.push(response.data[i].locality); }
That could be simplified using a for...of loop - e.g.
for (const datum of response.data) {
this.l_options.push(datum.locality);
}
While there may be a slight performance hit due to the iterator it is simpler to read.
Answered by Sᴀᴍ Onᴇᴌᴀ on December 21, 2021
You can use :old="{{ json_encode(Session::getOldInput()) }}" to get old values from session. Other than that try to avoid props like: p_old they break 2 principles:
When you define props as object try to be explicit about type: String, Array etc.
Answered by VeeeneX on December 21, 2021
Get help from others!
Recent Questions
Recent Answers
© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP