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

fix: make country as dropdown and add validation for gender #3809

Merged
merged 1 commit into from
Jan 16, 2020
Merged

fix: make country as dropdown and add validation for gender #3809

merged 1 commit into from
Jan 16, 2020

Conversation

maze-runnar
Copy link
Contributor

@maze-runnar maze-runnar commented Jan 15, 2020

Fixes: #3806
Fixes: #3780
Fixes: #3406

Short description of what this resolves:

In the attendee form validation for gender field was not working so i added it's validation identifier in
gender field.
The second issue was to make country field as drop-down, as it was text before.i added validation for it too. Also refactor the code little for readibility.

  1. when gender field is field -
    Screenshot from 2020-01-15 16-31-52
    2 - when gender field is not required -
    Screenshot from 2020-01-15 16-50-33
    3 - country drop-down with flags -
    Screenshot from 2020-01-15 16-55-18
    4 - validate gender when required and not given with country -
    Screenshot from 2020-01-15 15-25-55

Checklist

  • I have read the Contribution & Best practices Guide.
  • My branch is up-to-date with the Upstream development branch.
  • The acceptance, integration, unit tests and linter pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@auto-label auto-label bot added the fix label Jan 15, 2020
Copy link
Member

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maze-runnar have you checked if the country and gender values are getting saved ?

Copy link
Member

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also for attendee country change type from text to select in custom-form.js mixin

@maze-runnar
Copy link
Contributor Author

maze-runnar commented Jan 15, 2020

also for attendee country change type from text to select in custom-form.js mixin

this is a seperate isuue #3406 .

@kushthedude
Copy link
Member

Change the value as suggested by @snitin115

@maze-runnar
Copy link
Contributor Author

also for attendee country change type from text to select in custom-form.js mixin

done 👍

@kushthedude
Copy link
Member

A working gif with showing country field is getting saved?

@maze-runnar
Copy link
Contributor Author

A working gif with showing country field is getting saved?

@kushthedude it's not saving like gender field, as mentioned in issue #3406.
This is not a part of this issue, i think.

@kushthedude
Copy link
Member

It is saving just not rendering, also part of same issue, involves same changes.
No need for another micro PR fix it in same

@maze-runnar
Copy link
Contributor Author

maze-runnar commented Jan 15, 2020

It is saving just not rendering, also part of same issue, involves same changes.
No need for another micro PR fix it in same

Done -
Screenshot from 2020-01-15 19-12-17

@maze-runnar
Copy link
Contributor Author

@kushthedude here is full flow of attendee please take a look : https://youtu.be/o3uhEUdhl_8

@snitin315
Copy link
Member

Add fixes #3406 in your description as well

snitin315
snitin315 previously approved these changes Jan 15, 2020
@kushthedude
Copy link
Member

Squash the commits, and provide a final GIF showing all these changes in action.

in attendee list

changing `enter` to `select` in validation prompt

adding validation for gender

changing country type to select

render select type fields in attendee-list and make country as a
drop-down
@maze-runnar
Copy link
Contributor Author

Squash the commits, and provide a final GIF showing all these changes in action.

@kushthedude this is final video after all changes : https://youtu.be/o3uhEUdhl_8

@kushthedude
Copy link
Member

@iamareebjamal There will be a need for migration in server side, As we have moved on from text to select in Country Field of Attendee dropdown ?

@iamareebjamal
Copy link
Member

Yes

@kushthedude
Copy link
Member

@maze-runnar Please make a migration for same

@maze-runnar
Copy link
Contributor Author

@maze-runnar Please make a migration for same

@kushthedude can you please do it for me on server side 🙏

@kushthedude
Copy link
Member

To be merged after fossasia/open-event-server#6746

@iamareebjamal
Copy link
Member

Could I search the countries before? Using autocomplete?

@kushthedude
Copy link
Member

kushthedude commented Jan 16, 2020

Could I search the countries before? Using autocomplete?

I dont understand before ? But you can search for countries using keywords

@iamareebjamal iamareebjamal changed the title fix: making country as dropdown and adding validation for gender fix: make country as dropdown and add validation for gender Jan 16, 2020
@iamareebjamal iamareebjamal merged commit 6b3d80d into fossasia:development Jan 16, 2020
@maze-runnar maze-runnar deleted the patch-5 branch January 16, 2020 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants