Add non-restrictive palette choices option #73
Conversation
There was a problem hiding this comment.
Please, could you rename the option to samples as discussed in #68 ?
Then I will test your PR as soon as possible, thanks in advance!
rniem379349
force-pushed
the
feature/add-non-restrictive-palette-choices
branch
2 times, most recently
from
118a0a2
to
32d64be
Compare
|
Done :) thanks for the quick response |
There was a problem hiding this comment.
This is out of scope for this PR... but as plus, when using choices the color-widget spectrum should be hidden, allowing the user to choose only from the choices, but this is hard to do even by dirty-patching the widget via css.
If you can try to do it it would be very appreciated.
colorfield/fields.py
Outdated
| if self.choices: | ||
| choices = self.get_choices(include_blank=False) | ||
| palette = [choice[0] for choice in choices] | ||
| if self.samples: |
There was a problem hiding this comment.
The choices options has been removed, this is not backward-compatible at all.
We should keep both options: choices (restrictive) and samples (not restrictive).
colorfield/fields.py
Outdated
| kwargs['widget'] = ColorWidget(attrs={ | ||
| 'default': self.get_default(), | ||
| 'format': self.format, | ||
| 'palette': palette, | ||
| }) | ||
| return super(ColorField, self).formfield(**kwargs) | ||
|
|
||
| def deconstruct(self): | ||
| name, path, args, kwargs = super().deconstruct() |
There was a problem hiding this comment.
Please pass the class and self to the super call for backward-compatibility.
rniem379349
force-pushed
the
feature/add-non-restrictive-palette-choices
branch
from
32d64be
to
9382e24
Compare
colorfield/fields.py
Outdated
| @@ -53,12 +55,23 @@ def __init__(self, *args, **kwargs): | |||
|
|
|||
| def formfield(self, **kwargs): | |||
| palette = [] | |||
| if self.choices and self.samples: | |||
| raise ImproperlyConfigured("You can only set one of 'choices' and 'samples' for ColorFields.") | |||
There was a problem hiding this comment.
Maybe is it more correct to raise this exception in the init method?
rniem379349
force-pushed
the
feature/add-non-restrictive-palette-choices
branch
from
9382e24
to
4f9ff26
Compare
addresses Issue #68
This PR proposes a new field option which works similarly to django's
choiceskwarg, the difference being that the field's values are not restricted to the given choices. This way, one can use both the color palette and the color picker. Until now, withchoicesdefined, the color picker is effectively useless, since we cannot choose values outside of the palette.The supplied screenshot shows this logic in action in a sample django project. It illustrates the ability to save a color value outside the predefined choices. I realise screenshots are not necessarily always accurate - there is always a possibility that I doctored/edited it - so I have also added a test that cleans a model instance with
palette_choicesset and a supplied value outside those choices. Feel free to run the code yourself as well and test it all out.I have replaced the
choicesoption with the new option here. Please let me know if we should keep the originalchoicesas well. Perhaps adding some logic to make the two options mutually exclusive would be a good idea.