anitab-org / vms

THIS PROJECT IS ARCHIVED. Volunteer Management System.
GNU General Public License v2.0
1 stars 4 forks source link

#1050 UI:Change behaviour of address field in signup admin page #1098

Closed PrernaSingh587 closed 3 years ago

PrernaSingh587 commented 3 years ago

Description

In Sign Up Admin, I have made the city and state input disabled until the country is filled first. The city is disabled until the state is filled first.

Fixes #1050

Type of Change:

How Has This Been Tested?

Checked in my local server. Works well according to me. I have handled all the scenarios. inputVMS

Checklist:

PrernaSingh587 commented 3 years ago

I guess this code has some unnecessary checks and can be improved. We can achieve the change in fewer lines of code. Furthermore, these changes dont seem to work properly

Alright , I Will make the required modification. Could you tell me what is not working properly with these changes? Do I need to put some more to the code apart from changing it to jQuery?

rkpattnaik780 commented 3 years ago

I guess this code has some unnecessary checks and can be improved. We can achieve the change in fewer lines of code. Furthermore, these changes dont seem to work properly

Alright , I Will make the required modification. Could you tell me what is not working properly with these changes? Do I need to put some more to the code apart from changing it to jQuery?

Initially the state and city dropdowns are not disabled.

PrernaSingh587 commented 3 years ago

I guess this code has some unnecessary checks and can be improved. We can achieve the change in fewer lines of code. Furthermore, these changes dont seem to work properly

Alright , I Will make the required modification. Could you tell me what is not working properly with these changes? Do I need to put some more to the code apart from changing it to jQuery?

Initially the state and city dropdowns are not disabled.

As per the GIF, it is happening in mine.

rkpattnaik780 commented 3 years ago

I guess this code has some unnecessary checks and can be improved. We can achieve the change in fewer lines of code. Furthermore, these changes dont seem to work properly

Alright , I Will make the required modification. Could you tell me what is not working properly with these changes? Do I need to put some more to the code apart from changing it to jQuery?

Initially the state and city dropdowns are not disabled.

As per the GIF, it is happening in mine.

It is rather weird, if I run the app and navigate to admin signup through homepage, the dropdowns are not disbled by default. Can you show me a gif showing the dropdowns just after navigating t that page?

PrernaSingh587 commented 3 years ago

I guess this code has some unnecessary checks and can be improved. We can achieve the change in fewer lines of code. Furthermore, these changes dont seem to work properly

Alright , I Will make the required modification. Could you tell me what is not working properly with these changes? Do I need to put some more to the code apart from changing it to jQuery?

Initially the state and city dropdowns are not disabled.

As per the GIF, it is happening in mine.

It is rather weird, if I run the app and navigate to admin signup through homepage, the dropdowns are not disbled by default. Can you show me a gif showing the dropdowns just after navigating t that page?

. . . . @rkpattnaik780 Hello. Sorry for the delay in responding. newTEXTREVIEW

rkpattnaik780 commented 3 years ago

Hello @psingh587 . Can you please check if all your commits have been pushed cause I am not able to see it locally after pulling your PR. Furthermore, do address the other review comments, give appropriate spacing link x = a and not x =a. Squash your commits as well.

rkpattnaik780 commented 3 years ago

@psingh587 any update?

PrernaSingh587 commented 3 years ago

@psingh587 any update?

Really sorry for the delay. There were some technical issues with my laptop. I will work upon this commit tonight and get back to you.

PrernaSingh587 commented 3 years ago

@rkpattnaik780 I have made the file changes. It handles all the scenarios. Please check.

codecov[bot] commented 3 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (develop@b58bc98). Click here to learn what that means. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop    #1098   +/-   ##
==========================================
  Coverage           ?   86.93%           
==========================================
  Files              ?       85           
  Lines              ?     4057           
  Branches           ?      237           
==========================================
  Hits               ?     3527           
  Misses             ?      458           
  Partials           ?       72           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b58bc98...b44ce93. Read the comment docs.

PrernaSingh587 commented 3 years ago

Hey! I have raised another PR and closing this one. The commits here went a bit bogus and I was not able to squash them.

1124