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

make fixed currency possible #720

Open
nerdoc opened this issue Jul 5, 2023 · 6 comments
Open

make fixed currency possible #720

nerdoc opened this issue Jul 5, 2023 · 6 comments

Comments

@nerdoc
Copy link
Contributor

nerdoc commented Jul 5, 2023

There are use cases where only one currency is supported under certain circumstances, e.g. the user in an app has chosen the currency in his settings. I would like to just allow this one currency in the MoneyWidget, but the code does not allow that. What is possible:

  • use MoneyField(default_currency="EUR") - which only selects EUR as default, but allows all other currencies too.
  • use MoneyField(currency_choices=[("EUR", _("Euro"))]) - which reduces the choices to one, but it is still a <select>.

in the MoneyField code, currency_field is hard coded:

currency_field = ChoiceField(choices=currency_choices)

So this does not allow any customization.
What I really want is something like this:
grafik

Like a fixed input-group-text, here in e.g. Bootstrap5. This could be made customizable.

For that, It would be enough to add code to MoneyWidget: if there is only one currency_choices, it should not be displayed as select - or at least, to be in sync with the form requirements, to disable the select, which has a similar effect, and would be clearer for the user - he is not tempted to click on the select - which has no choices anyway.

@nerdoc
Copy link
Contributor Author

nerdoc commented Jul 5, 2023

It is possible at least a bit, by defining a default_widget:

amount = MoneyField(
    currency_widget=Select(choices=[("EUR", _("Euro"))], attrs={"disabled": True}),
)

But you have to to this fo each field manually, and it somehow doesn't send the value of the field back to the form, so cleaned_data["amount"] is not available any more. This is not usable.
It would be much more convenient to let the widget sense the singularity of the choices and disable the select hence.

@benjaoming
Copy link
Contributor

@nerdoc Maybe a project-wide DJMONEY_CURRENCY_WIDGET setting could be interesting. It could be a string with a dotted path of a custom class that should always be used.

It would be much more convenient to let the widget sense the singularity of the choices and disable the select hence.

I think that this behavior could be undesired for projects where the currency choices are dynamic. But if you can configure a currency widget for the entire project, maybe that will work?

Like a fixed input-group-text, here in e.g. Bootstrap5. This could be made customizable.

This level of customization won't work as a project setting, since the widget is also used in the Django admin which doesn't have Bootstrap. In these cases, it's probably better to find another way to override MoneyField's composite widget.

Forms always require a certain amount of customization work. But there are many patterns to achieve reusability across entire sites, without affecting the Django Admin. You could consider working with django-crispy-forms form this.

@nerdoc
Copy link
Contributor Author

nerdoc commented Jul 7, 2023

@nerdoc Maybe a project-wide DJMONEY_CURRENCY_WIDGET setting could be interesting. It could be a string with a dotted path of a custom class that should always be used.

Yes, that would be a possibility, because then it can be fully customized per project. Even if customizing a widget is more work than a sensitive setting.

Like a fixed input-group-text, here in e.g. Bootstrap5. This could be made customizable.

This level of customization won't work as a project setting, since the widget is also used in the Django admin which doesn't have Bootstrap. In these cases, it's probably better to find another way to override MoneyField's composite widget.

Sure. I didn't mean using BS hard coded in django money ;-)
I ran into problems when I changed the widget to disabled state too, this is far more interesting, and could solve that problem here maybe even better.

First, I just tried to disable the widget:

class SomeForm(forms.ModelForm):
    amount = MoneyField(
        # currency_widget=Select(choices=[("EUR", _("Euro"))], attrs={"disabled": True}),  # <- buggy
        currency_widget=Select(choices=[("EUR", _("Euro"))]),
    )
    def __init__(self, **kwargs):
        super().__init__(**kwargs)
        self.fields["amount"].widget.attrs["autofocus"] = True  # works perfectly
        self.fields["amount"].widget.widgets[1].attrs["disabled"] = True  # <- buggy

in these two ways shown above, the second widget gets disabled - which is my desired behaviour if there is only one currency choice. This can be made dynamically too.

It works for rendering (and yes, I aready am using crispy forms). The problem here is that I always get a form error when posting - that the whole amount field is required. It seems that if the field is disabled, it does not return a value (here as "amount_1") to the view, which lets the cleaning fail.

So easier than creating a custom project wide custom widget implementation would IMHO be allowing users customize the widget.

Maybe this could even be thought further and letting the user directly access the widget using self.fields["amount"].currency_widget instead of self.fields["amount"].widget.widgets[1]. But that's just syntactic sugar.

I didn't examine the internals of MoneyField that far, but I could not find a cause for letting the value vanish when the field is disabled. Maybe this is Django's normal behaviour, but as the currency is a required field it could be fixed in a clean() method too from the defaults if there are any...

Use case:

  • user chooses to restrict currencies to one (EUR), even dynamically. Let's say, in one specific case a transaction is only allowed in EUR for that user, so the field should in this case just show one currency.

In my case, it looks like that (for a simple cashbook) - I already implemented the disabled state here:
grafik

This is what it SHOULD look like. But with above code, it does not work, as stated. Instead, I did a trick and used

self.fields["amount"].widget.widgets[1].attrs["hidden"] = True 

So the currency_field is hidden, but still in the form (hence returning a value and preventing the required constraint).
Then, to display the "fake" currency, I just used crispy and an AppendedText("amount", "EUR")), - so it looks like a disabled checkbox.

The important thing is that it should not have a tab stop - the cursor should go directly to the next (comment) field when the user presses [TAB] and should not land on the currency field.

I know, this all sounds a bit over-engaged - but I like optimizing to death :-)

@benjaoming
Copy link
Contributor

Instead of using the disabled attribute, would it be possible to use either a HiddenInput widget or the readonly attribute?

@nerdoc
Copy link
Contributor Author

nerdoc commented Jul 9, 2023

The select tag in HTML doesn't have a readonly attribute... And HidenInput widged is used if the disabled flag is set within Django forms. And a per-project HiddenInput setting is not good if you want a dynamic select, on a per-form base e.g.

@nerdoc
Copy link
Contributor Author

nerdoc commented Jul 9, 2023

What I have to admit, that this: works, at least of skipping the cursor:

    amount = MoneyField(
        currency_widget=Select(choices=[("EUR", _("Euro"))], attrs={"disabled": True}),
    )

It's just that

  • the form doesn't doesn't receive any value from the currency widget
  • The currency widget is rendered very badly IMHO - below the number input - but this is not your business here...

It just think that
grafik
is better than
grafik

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants