timonweb / django-tailwind

Django + Tailwind CSS = đź’š
https://django-tailwind.readthedocs.io
MIT License
1.47k stars 92 forks source link

JIT is coming. Django-Tailwind 2.0 too. #56

Closed timonweb closed 3 years ago

timonweb commented 3 years ago

Hi there,

Anyone brave enough to test the latest state of the master branch? Especially the upgrade instructions here: https://django-tailwind.readthedocs.io/en/latest/migration.html.

@justinmayer, do you want to play? ;)

justinmayer commented 3 years ago

@timonweb: <Dusts off shoulders…> Challenge accepted!

The main issue I keep running into is:

> pip install -U https://github.com/timonweb/django-tailwind/archive/refs/heads/jit.zip
> python manage.py tailwind init
Unknown command: 'tailwind'
Type 'manage.py help' for usage.

I can't quite figure out what is causing this. Any suggestions?

timonweb commented 3 years ago

@justinmayer make sure you've added tailwind to INSTALLED_APPS

timonweb commented 3 years ago

I just upgraded my blog to Django-Tailwind v2.0 (https://timonweb.com/), and it worked like a charm. My optimized stylesheet size even decreased by 2kb!

I followed the upgrade instructions to catch any flaws and haven't noticed any. So if you guys could just go the same path and give me feedback would be great. I'm planning to make the public release next week.

justinmayer commented 3 years ago

My issue above was due to creating a fresh virtual environment in which DJANGO_SETTINGS_MODULE was not defined, and in which manage.py had a default DJANGO_SETTINGS_MODULE value that didn't actually point to an existing settings file. After fixing that, I took some notes on my upgrade journey…

> python manage.py tailwind init

AttributeError: 'Settings' object has no attribute 'BASE_DIR'

This is the first time I have seen an app depend on that specific variable, which I had always assumed was for internal use when referring to paths relative to project root. In my settings file, I define that as PROJECT_ROOT instead. For the moment, I worked around this problem by adding: BASE_DIR = PROJECT_ROOT

In step 6, I've never changed anything in theme/static_src/src/ and found that the contents of the legacy styles.scss file to be the same as the new version's styles.css. So I don't think there was anything I needed to do for this step. Might be good to clarify that, for this step, there's no need to do anything if there haven't been any changes to theme/static_src/src/. (Assuming, of course, that I have understood this step correctly.)

When running python manage.py tailwind start for the first couple of times, I got errors like this: https://dpaste.org/y8ga Those errors went away for subsequent invocations, but I don't know why.

I got an error when loading my app due to a template tag I had added to the theme but didn't move over. It might make sense to add language to the upgrade instructions that remind folks to copy over anything else that was added, probably by using a diff tool to compare the directories.

Everything else seemed to proceed as expected. When I ran runserver to look at my web app, however, the styles were not fully preserved for some unknown reason. You can see the differences below:

Django-Tailwind 1.2:

django-tailwind-1.2

Django-Tailwind 2.0:

django-tailwind-2.0
timonweb commented 3 years ago

Hi @justinmayer thank you for jumping in and your notes.

re BASE_DIR - isn't it a standard part of the settings.py? I need it to undertand where there root of the project is. Probably there's another way to do that. Need to check how python manage.py startapp does it.

re error dump. I've added nodemon to watch config file updates. Probably it had some issues at the beginning, can't say why atm.

re styles that are off. First of all, we now include @tailwindcss/forms by default, which you might haven't used before. If you don't that plugin, uncomment it in tailwind.config.js. Secondly, it looks like JIT doesn't see all paths where you use Tailwind classes. Could you check paths under purge in tailwind.config.js?

justinmayer commented 3 years ago

re BASE_DIR - isn't it a standard part of the settings.py?

Running the django-admin startproject foo project scaffolding task does indeed generate a settings.py file that defines a BASE_DIR variable, but I don’t think that means third-party apps can reliably depend on its existence. I’ve never seen a third-party app depend on it, and I’ve never seen Django documentation that indicates third-party apps can assume it will always be present.

I need it to understand where there root of the project is. Probably there's another way to do that.

Understandable. I think it’s okay for third-party apps to check whether BASE_DIR is defined and to use it if the specified directory exists, but I think it’s also important to have a fall-back function to determine the project root if either of those aforementioned two conditions fail.

re styles that are off. First of all, we now include @tailwindcss/forms by default, which you might haven't used before. If you don't that plugin, uncomment it in tailwind.config.js.

That was indeed part of why the styling looked a bit different. Up until now I have been using Crispy Forms and its Crispy Tailwind pack, which post-upgrade was being overridden by @tailwindcss/forms. Uncommenting the latter in tailwind.config.js produced the previous rendering. This brings up a topic I’ve long meant to raise — how to use the two projects together most effectively — but perhaps I’ll do that in the context of another issue.

Secondly, it looks like JIT doesn't see all paths where you use Tailwind classes. Could you check paths under purge in tailwind.config.js?

I was able to track down the transparent button and missing link color for "Forgot your password?" to my custom Tailwind template tag filters. One of the questions I’ve not yet answered (and the subject of another issue I’ll bring up separately) is how best to extract common components (buttons, form elements, badges, etc.) in a Django-Tailwind context. Up until and including v1.2, I have taken the template partial approach by experimenting with Django’s @stringfilter decorator to define button_class() and text_link_class() functions (in theme/templatetags/components.py) that can be called from templates via, for example: <button class="{{ 'indigo' | button_class }}">

As far as I can tell from looking at the rendered HTML, that functionality seems to continue to work just fine even after upgrading to Django-Tailwind 2.0/JIT — all the expected classes are there, but as seen in the above screenshots, the button is transparent and the link is black instead of indigo. Both of those problems are remedied if I replace the template tag filters with the exact same classes they render, so clearly there is something about the template tag filters that's causing this problem. Since I don't fully understand how the purge path business works, I don't know how to resolve this problem. Any ideas?

timonweb commented 3 years ago

Justin, thank you for the input!

Yes, let's discuss other points in separate issues. As for the purge setup, please check this: https://tailwindcss.com/docs/optimizing-for-production#writing-purgeable-html it gave me a good idea about do's and dont's.

timonweb commented 3 years ago

FYI, just released public beta here: https://pypi.org/project/django-tailwind/2.0.0b1/

simkimsia commented 3 years ago

Sorry i cannot find the branch https://github.com/timonweb/django-tailwind/tree/jit anymore. I'm going to install 2.0.0b1

Then do I need to redo the init step?

I already have a theme from my previous step

timonweb commented 3 years ago

@simkimsia thanks for diving in! The jit branch is gone, everything is in the master. You can either redo the init step or follow the migration instructions here: https://django-tailwind.readthedocs.io/en/latest/migration.html

Let me know how it went, any feedback is very much needed.

Thanks!

simkimsia commented 3 years ago

i'm now rebuilding my entire docker.

because originally i wrote in my requirements.txt simply

django-tailwind # without stating the exact version

I wasn't sure what version of dt i used to do my original init

and also i don't know whether i should upgrade

So I have no easy way to figure out.

Maybe there's already such a thing, and I'm asking a dumb question, but is it possible to have a

python manage.py tailwind version

which then states the version of django-tailwind? Not sure whether would be useful to be more verbose to also state the version of tailwind used. but the minimal is to have django-tailwind version

Giving you context to help you better understand my level of proficiency,

  1. I'm more of a django dev than a frontend dev in terms of tech skills. So as much as possible I prefer django style way of doing things.
  2. I'm more of a biz owner than a dev temperamentally even tho I can write code. So I'm not super versed in django. I'm average.

Thank you

simkimsia commented 3 years ago

Sorry I just finished the rebuild of docker,

and I tried and I found that

/code # python manage.py tailwind --v
usage: manage.py tailwind [-h] [--no-sync] [--no-jit] [--no-input]
                          [--app-name APP_NAME] [--version] [-v {0,1,2,3}]
                          [--settings SETTINGS] [--pythonpath PYTHONPATH]
                          [--traceback] [--no-color] [--force-color]
                          [--skip-checks]
                          label [label ...]
manage.py tailwind: error: ambiguous option: --v could match --version, --verbosity
/code # python manage.py tailwind --version
3.2b1

SO:

  1. there's indeed a --version flag. Apologies
  2. would be great if the --version flag also states the existing django-tailwind version as well on top of the django framework
simkimsia commented 3 years ago

Sorry for the multiple responses. I'm realizing the ways that are helpful for newbies to django-tailwind like me.

You can either redo the init step or follow the migration instructions here: https://django-tailwind.readthedocs.io/en/latest/migration.html

Do you mind if there's a way to indicate the version of the django-tailwind used to generate the theme and have that indication inside the theme folder itself?

This makes it more explicit. Maybe just have a README.md inside the theme that states the version of django-tailwind used to generate the theme and some helpful links including this migration link you gave.

timonweb commented 3 years ago
  1. --version is the global Django flag for all manage.py commands. It shows you the Django version
  2. Run pip show django-tailwind to find out the current version
  3. Re putting a version number into the generated app. That's a good idea, I could place it somewhere in package.json
  4. I hope that going forward v2 there won't be a need to regenerate apps anymore.
simkimsia commented 3 years ago

no further questions 🙏 for this issue per se

timonweb commented 3 years ago

v2.0 has been released.