-
Notifications
You must be signed in to change notification settings - Fork 2
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
Allow to define radio button form via settings #1
Conversation
registrasion/forms.py
Outdated
VoucherForm = util.get_object_from_name(settings.VOUCHER_FORM) | ||
except: | ||
VoucherForm = _VoucherForm | ||
def get_form_class_from_settings(config_name, default): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might suggest moving this up to the top of the file for discoverability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in da25274
registrasion/forms.py
Outdated
@@ -168,7 +168,7 @@ def ProductsForm(category, products, sold_out_products=None, disabled_products=N | |||
cat = inventory.Category | |||
RENDER_TYPES = { | |||
cat.RENDER_TYPE_QUANTITY: _QuantityBoxProductsForm, | |||
cat.RENDER_TYPE_RADIO: _RadioButtonProductsForm, | |||
cat.RENDER_TYPE_RADIO: get_form_class_from_settings('RADIO_BUTTON_PRODUCTS_FORM', _RadioButtonProductsForm), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
two things:
- can we prefix these settings with
REGISTRASION_
- should we apply the same logic to all render types while we're here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with both!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great @berinhard! We probably want a matching PR to update the Attendee Profile and Voucher form setting names in the pycon2021 codebase to merge with this one, right? |
Sure, here you are: https://github.com/psf/pycon2021/pull/13/commits/4c1123d20043b009c80c0c82ecb780eac34a587a It's the most recent one in the PR #13. |
@berinhard can we break that out separately to merge this first? |
Not sure if I followed you... Should I split new settings variables in 2 PRs, one for registrasion and other for PyCon? |
just a PR for the commit referenced: https://github.com/psf/pycon2021/commit/4c1123d20043b009c80c0c82ecb780eac34a587a That will allow us to merge this and that PR simultaneously then the rest of https://github.com/psf/pycon2021/pull/13/ later after more thorough review. |
Here you are @ewdurbin: |
perfect. thank you! |
This was required because I needed to style how the form are being rendered in PyCon 2021 website.