django-crispy-forms / crispy-tailwind

A Tailwind template pack for django-crispy-forms
MIT License
331 stars 56 forks source link

Less opiniated defaults #90

Open tissieres opened 3 years ago

tissieres commented 3 years ago

Hello, I played a little bit with your app today and I think the default classes added to elements are a little too opiniated. For example the select field add a SVG handle and multiple classes for the colors of the border or the text. I usually style my inputs starting from this @tailwindcss/forms as recommended by the authors of TailwindCSS. It doesn't play well with your templates. The select element get multiple handles for example. One solution would be to override the templates for these elements, but it is a lot of work. Would it be possible to add these default styles through the CSSContainer for example? Or provide another way to start from less opiniated templates? Thanks for your work! Cedric

smithdc1 commented 3 years ago

Hi @tissieres

Thanks for your feedback I think what we're running into here is that Tailwind by its nature is designed to be customised. We can provide some styles out of the box but I suspect folk will want to customise much more than with the bootstrap packs.

Having said that, if we can give a form that looks OK "straight out of the box" but allows for easier customisation then we should do this. Maybe the tailwind form plugin could help here? https://github.com/tailwindlabs/tailwindcss-forms

What do you think?

tissieres commented 3 years ago

Hi @smithdc1

In my opinion, this app should ask users to install and enable the tailwindcss forms plugin beforehand and then provide light customisations that can be overridden through the CSSContainer for example. The simple style shown in this demo https://tailwindcss-forms.vercel.app/ could be a good example. It would remove the conflict between the tailwindcss form plugin and the templates given in this app.

amitjindal commented 2 years ago

Hi @smithdc1

I also have a similar problem. I have tried an experiment. Please see if this approach works for the long term and I will be happy to submit a PR for the same.

Problem 1: It is very difficult to override default styling especially when we use third party packages.

I would propose that this be provided via settings. To do this override the default styles in crispy_tailwind/templatetags/tailwind_field.py file from settings. Even partial settings will work.

Example code:

Change:

    default_container = CSSContainer(default_styles)

To:

    default_container = CSSContainer({**default_styles, **getattr(settings, "CRISPY_TAILWIND_STYLE", {})})

where we define in project settings:

CRISPY_STANDARD_INPUT = (
    "h-8 bg-gray-200 border-gray-500"
)

CRISPY_TAILWIND_STYLE = {
    "text": CRISPY_STANDARD_INPUT,
    "number": CRISPY_STANDARD_INPUT,
    "radioselect": "",
    "email": CRISPY_STANDARD_INPUT,
    "url": CRISPY_STANDARD_INPUT,
    "password": CRISPY_STANDARD_INPUT,
    "hidden": "",
    "multiplehidden": "",
    "file": "",
    "clearablefile": "",
    "textarea": CRISPY_STANDARD_INPUT,
    "date": CRISPY_STANDARD_INPUT,
    "datetime": CRISPY_STANDARD_INPUT,
    "time": CRISPY_STANDARD_INPUT,
    "checkbox": "checkbox",
    "select": "select",
    "nullbooleanselect": "select",
    "selectmultiple": "select",
    "checkboxselectmultiple": "",
    "multi": "",
    "splitdatetime": "text-gray-700 bg-white focus:outline border border-gray-300 leading-normal px-4 "
                     "appearance-none rounded-lg py-2 focus:outline-none mr-2",
    "splithiddendatetime": "",
    "selectdate": "",
    "error_border": "border-red-500",
}

This allows very easy customization across a wide style of changes.

Problem 2: Some templates are not using the specified styles

I came across this problem in select. File: crispy_tailwind/templates/tailwind/layout/select.html. The styling is done directly. Even the default_styles mentioned above is not being used.

I am not too familiar with how crispy forms in general works to understand the reason but I figured it must be as you do not want to use the django provided template. And perhaps there is no easy way to apply the CSSContainer style in template directly.

To overcome this I changed the instance of CSSContainer to a global object and using a template_tag to apply the style now defined via settings.

Changes:

  1. Register a new filter
@register.filter
def tailwind_field_class(field):
    """
    Returns field class from defaults.
    """
    return f" {tailwind_container.get_input_class(field)}"
  1. Use the filter in select template:

    @@ -1,8 +1,8 @@
    -{% load crispy_forms_filters %}
    +{% load crispy_forms_filters tailwind_field %}
    {% load l10n %}
    
    <div class="relative">
    -<select class="{% if field.errors %}border border-red-500 {% endif %}bg-white focus:outline-none border border-gray-300 rounded-lg py-2 px-4 block w-full appearance-none leading-normal text-gray-700" name="{{ field.html_name }}" {{ field.field.widget.attrs|flatatt }}>
    +<select class="{% if field.errors %}border border-red-500 {% endif %} {{ field | tailwind_field_class }}" name="{{ field.html_name }}" {{ field.field.widget.attrs|flatatt }}>
     {% for value, label in field.field.choices %}
         {% include "tailwind/layout/select_option.html" with value=value label=label %}
     {% endfor %}

Full change in tailwind_field.py to support both of these

@@ -76,42 +76,54 @@ def pairwise(iterable):
     return zip(a, a)

+@register.filter
+def tailwind_field_class(field):
+    """
+    Returns field class from defaults.
+    """
+    return f" {tailwind_container.get_input_class(field)}"
+
+
+base_input = (
+    "bg-white focus:outline-none border border-gray-300 rounded-lg py-2 px-4 block w-full "
+    "appearance-none leading-normal text-gray-700"
+)
+
+default_styles = {
+    "text": base_input,
+    "number": base_input,
+    "radioselect": "",
+    "email": base_input,
+    "url": base_input,
+    "password": base_input,
+    "hidden": "",
+    "multiplehidden": "",
+    "file": "",
+    "clearablefile": "",
+    "textarea": base_input,
+    "date": base_input,
+    "datetime": base_input,
+    "time": base_input,
+    "checkbox": "",
+    "select": "select AMIT",
+    "nullbooleanselect": "",
+    "selectmultiple": "",
+    "checkboxselectmultiple": "",
+    "multi": "",
+    "splitdatetime": "text-gray-700 bg-white focus:outline border border-gray-300 leading-normal px-4 "
+                     "appearance-none rounded-lg py-2 focus:outline-none mr-2",
+    "splithiddendatetime": "",
+    "selectdate": "",
+    "error_border": "border-red-500",
+}
+
+tailwind_styles = {**default_styles, **getattr(settings, "CRISPY_TAILWIND_STYLE", {})}
+tailwind_container = CSSContainer(tailwind_styles)
+
+
 class CrispyTailwindFieldNode(template.Node):

-    base_input = (
-        "bg-white focus:outline-none border border-gray-300 rounded-lg py-2 px-4 block w-full "
-        "appearance-none leading-normal text-gray-700"
-    )
-
-    default_styles = {
-        "text": base_input,
-        "number": base_input,
-        "radioselect": "",
-        "email": base_input,
-        "url": base_input,
-        "password": base_input,
-        "hidden": "",
-        "multiplehidden": "",
-        "file": "",
-        "clearablefile": "",
-        "textarea": base_input,
-        "date": base_input,
-        "datetime": base_input,
-        "time": base_input,
-        "checkbox": "",
-        "select": "",
-        "nullbooleanselect": "",
-        "selectmultiple": "",
-        "checkboxselectmultiple": "",
-        "multi": "",
-        "splitdatetime": "text-gray-700 bg-white focus:outline border border-gray-300 leading-normal px-4 "
-        "appearance-none rounded-lg py-2 focus:outline-none mr-2",
-        "splithiddendatetime": "",
-        "selectdate": "",
-        "error_border": "border-red-500",
-    }
-
-    default_container = CSSContainer({**default_styles, **getattr(settings, "CRISPY_TAILWIND_STYLE", {})})
+    default_container = tailwind_container

     def __init__(self, field, attrs):
         self.field = field

I have the changes in a fork. If you would like to review, here they are: https://github.com/amitjindal/crispy-tailwind/tree/feature-customizable

Do you think they make sense for the overall design?

-Amit

GitRon commented 5 months ago

Been a while since the last lifesign in here. Anybody feels like keeping this issue open?

Thutmose3 commented 4 months ago

I think it's a good idea, because now it seems that the styles are hardcoded? And being able to set your tailwind classes in settings.py would be great. Like django_tables_2 has DJANGO_TABLES2_TABLE_ATTRS: https://django-tables2.readthedocs.io/en/latest/pages/custom-rendering.html

Thutmose3 commented 4 months ago

Created a PR that allows us to override the defaults in settings: https://github.com/django-crispy-forms/crispy-tailwind/pull/163 Based on @amitjindal idea