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

Less opiniated defaults #90

Open
tissieres opened this issue Apr 6, 2021 · 6 comments
Open

Less opiniated defaults #90

tissieres opened this issue Apr 6, 2021 · 6 comments

Comments

@tissieres
Copy link

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
Copy link
Member

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
Copy link
Author

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
Copy link

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
Copy link
Contributor

GitRon commented Feb 8, 2024

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

@Thutmose3
Copy link
Contributor

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
Copy link
Contributor

Created a PR that allows us to override the defaults in settings: #163
Based on @amitjindal idea

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

5 participants