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

To Do #1

Open
14 of 25 tasks
smithdc1 opened this issue Apr 26, 2020 · 19 comments
Open
14 of 25 tasks

To Do #1

smithdc1 opened this issue Apr 26, 2020 · 19 comments

Comments

@smithdc1
Copy link
Member

smithdc1 commented Apr 26, 2020

Hi @carltongibson @bittermandel

I've pushed an initial commit based upon the pull request to django-crispy-forms master from last week. It's not much more than a POC at the moment, with the current tests passing.

Much work to be done; here are my current thoughts. This won't be a complete list but is enough to be getting on with for now - and probably we can get an inital release out before completing all of them.

EDIT: I've updated the below to split into sections to give some sort of percieved importance. I'll keep this list live for now.

Completed

  • Template imports - needs to be {% load tailwind_field %}
  • Setup GitHub actions for testing, isort, black, flake8
  • Include a default rendering option (something like {{ form|crispy }})
  • Setup development environment with crispy-test-project
  • Add oppinionated rendering to |crispy
  • Add an example image to README
  • code coverage + template coverage? (seems to be a bug with codecov, see PR)
  • GH Action - let's try and cache the dependencies to stop pulling all the dependencies every time. e.g. Django get's downloaded 14 times for every pull request that is merged = circa.105mb. Note: cache is 41mb per run (500mb for a merged PR?)
  • Docs why are images not showing (do we 'just' need to use RTD?). Note: Img files need to be lower case.
  • Should we publish something to PyPI now to get the name?
  • Custom objects - e.g. prepend/postpend
  • Find out why codecov went missing again. (It's reporting https://codecov.io/gh/django-crispy-forms/crispy-tailwind, but it's not commenting on PRs any more)
  • Non form errors (e.g. passwords do not match?)

Sooner ™️

  • Review project strucutre - can someone review how I've setup the licence, setup.py etc?
  • Add more tests for layout
  • Add test for Field layout object
  • Docs
  • Fix flake8 - we're running well over on line length. Also why isn't it failing!?

Later

  • What to do about radio / checkbox
  • Put tests into more logical structure.
  • Put note on django-crispy-forms that this template pack exists
  • Review to do list items I've left in the code
  • Tidy up code base
  • Review GH Actions - are they running as expected?
  • Refactor field / label class when using crispy filter. (don't need to add classes to tailwind_filters - can we remove this file?!)
@bittermandel
Copy link
Contributor

Hey! I haven't had time to look into this yet, I will try to get some time during this week :)

@bittermandel
Copy link
Contributor

Hey! I'm just looking into this and getting acquainted with how crispy forms templates work. Do you wanna have a chat when you're online? Would help me get up to speed!

@smithdc1
Copy link
Member Author

Hey! I'm just looking into this and getting acquainted with how crispy forms templates work. Do you wanna have a chat when you're online? Would help me get up to speed!

Would be good to have a chat, timing is a bit tricky for me at the moment given the current circumstances it's a muddle of work and family at the moment. I have some time, but to say "when" is the tricky bit.

I'm doing a bit of work on the |crispy filter which will help, I think. Especially if I can put some tests together.

In the meantime the short* answer is something like this:

  • Field.html is the key template. This is template is called for each field in the layout
  • Logic here will try and render the required layout, usually a combination of
    • Div (or other tag) arround the field
    • A label , with classes
    • An input, with classes
  • However, radios / checkboxes are usually a bit more tricky, especially the multiple variants. For these we send the logic off to more complex templates in the layout folder. Alignment issues here are often an issue (try looking at the checkbox template for bootstrap 4! 😱 ). Interested in how complex we make this bit as radio/checkbox don't look great as standard.
  • Then we have the layout objects, e.g. the "inline" radio / checkbox. These have specific templates associated with these classes and are contained within the layout folder. We can continue to build these out overtime, Bootstrap has many of these, I've limited them down somewhat already, could go a bit further. e.g. I've left 'alerts' in.
  • Help / Error have their own templates for rendering these
  • Buttons... going to go with a "not sure". I tend to add classes and they work, it's somewhere between "form actions" and "base input".
  • Some items I suggest we leave for now, file fields and MultiValueFields fall into this category for me.

The key 'gotcha' that I've encounted is miss understanding how the inputs are rendered. We're using Django for this, and passing in classes to the widget. (i.e. we're not using base input). We use the render function of CrispyTailwindFieldNode in tailwind_field.py to add the required CSS classes for inputs.

For other template packs they are quite oppinionated. e.g. for bootstrap4 it's form-control here and there. For Tailwind I think we need a bit more flexibility so have created CSSContainer which we attach to the helper and then dig out the right classes in the templates. Not sure if this is the right apprach and even less sure about the name.

I'd also like to add an oppinionated version to get people with a "good enough" form style that "just works"™️. I think we can do this by adding some classes to the labels and having a default "CSSContainer" or set of styles we can add. Really value your oppinion here as otherwise I'm copy and pasting off the examples on their website (which may be ok?).

Finally, massive amounts of imposter syndrome at my end, my experiance of Tailwind is close to none. But "it's always a template issue", and we can solve these.

'* Not so sure I'm good at the 'short' answer thing...

@smithdc1
Copy link
Member Author

Hi @bittermandel

I've added a crispy filter for tailwind. Carlton did mention using .as_p() but I don't quite know in my head how this would work, maybe if people are familar with crispy-forms then maybe |crispy is ok, it's fairly old?

Here is the test. We therefore now have a working example where a standard form can have |crispy added and a nice form is rendered.

The test is pretty ugly but it atleast it shows the full form being rendered. We have classes being added to labels, divs and inputs. This feels like a good step forward.

Now it's some design work, setting up the defaults which we think are appropriate.

Do you have any views on this?

@smithdc1 smithdc1 pinned this issue Apr 30, 2020
@bittermandel
Copy link
Contributor

Hey! Thanks for the elaborate answers, finally got some time to look through all of this.

I was quite confused over how the filters work, how labels are applied and how the filter is used. This is a bit clearer now with the test you've written, turned out that's a great way for me to learn :) Especially how, like you said, we use Django to render each input using widgets.

I am not sure what alternatives that could be done around CSSContainer. What I feel is that having really great default styling for each element that we support is key! It should be very straight forward to just throw form|crispy in a template and render a decently pretty and functional form 🙂

I've been using Tailwind quite a bit this year and usually end up finding inspiration in https://tailwindui.com and the free examples. I am not sure if the free examples is something we are allowed to re-distribute, but if we can, I believe we can use them for all default stylings.

I'm currently working on a hobby project and rendering all my forms using this project. I was thinking I could provide a few PRs containing good looking designs and tests as I progress, most importantly around selects and error messaging.

Hoping to send in a few PRs today with the styling 👍

@bittermandel
Copy link
Contributor

Are we planning to support rendering using tags in the future as well? I was looking into using Helpers to render extra submit buttons etc, but this doesn't seem to be working out of the box with how CrispyTailwindFieldNode is written today.

@smithdc1
Copy link
Member Author

smithdc1 commented May 1, 2020

Are we planning to support ... render extra submit buttons etc, but this doesn't .... work...... CrispyTailwindFieldNode

We should support this.

On the pull request I did on django-crispy-forms I got some buttons working where i added the CSS classes in the helper. Not in front of my laptop right now to have a deeper look (I'm surprised that rendering buttons try and call CrispyTailwindFieldNode).

@bittermandel
Copy link
Contributor

About CrispyTailwindFieldNode, this is likely an misunderstanding on my part of how crispy-forms implements filters and tags.

@smithdc1
Copy link
Member Author

smithdc1 commented May 2, 2020

Hi all

So I had some time to reflect on this earlier today and I think there are 4 stages at which people should be able to use this template pack:

  1. |crispy filter. Lowest cost of entry, just use the filter and you get a nice Trailwind form with our oppinions on how it should look.
  2. {% crispy %}. Next stage alows the benefits of the helper to layout the form.
  3. Custom-Forms . In addition to the {% crispy %} form - you can switch on an additional flag (suggest we use use-custom-control) which has the added benefit of better styling of radio/checkbox using custom-forms. (Not sure how this works - they don't seem to have a css cdn for this? )
  4. Full custom - Take control of CSSContainer and use this to help design your form.

Thoughts?

@smithdc1
Copy link
Member Author

smithdc1 commented May 9, 2020

Hi all

Hope you are all well, this week as been a bit crazy at my end so have been a bit quiet.

I've setup the Tailwind test site over at crispy-test-project on the tailwind branch
https://github.com/django-crispy-forms/crispy-test-project/tree/tailwind

You will need to install the dependencies of django-crispy-forms and this package for it to work (I installed it with pip install -e 'path-to-crispy-tailwind

I've also added a |crispy form as we're working on that currently. This is what the tailwind page currently looks like:

We're currently working on the bit with the red box round it - the other bits work as I've passed a styling into the form-helper (as per original PR to crispy-forms).
screencapture-127-0-0-1-8000-tailwind-2020-05-09-10_02_05

@smithdc1
Copy link
Member Author

A quick update on docs. I've added a GitHub action that on push to the master branch builds and deploys the docs to gh-pages branch. You can see the docs here.

@marcorichetta
Copy link

Hi! I came here from Will Vincent tweet. I think crispy-forms is a great project for the Django ecosystem and I tried Tailwind sometime ago and I really liked it.

We're currently working on the bit with the red box round it - the other bits work as I've passed a styling into the form-helper (as per original PR to crispy-forms).

@smithdc1 Could you indicate what are you working on exacty?

  • For example, I ran the test project on my pc and I saw the Grouped checkboxes still look as in the image above. Is this something to fix?
  • Also, is there some reason the current PRs are not merged?

Thanks for all the initial work done in this project! 👍

@smithdc1
Copy link
Member Author

Hi @marcorichetta 👋

Thank you for your interest in this package. To answer your questions:

What am I working on

Currently their are two main things. (We've moved on a bit since the image above, the standard widgets are now rendered nicely 'out of the box').

  1. Test coverage - it is very low at the moment. I'm hopeful most of it should just work at this point but...

  2. Docs - In particular the layout docs. Need to explain each layout item, where it is sourced from, what it looks like etc.

Grouped Checkboxes

Grouped checkboxes is a much wider issue for crispy-forms; they look like this on bootstrap as well. Not yet solved why crispy-forms outputs the groups instead of individual items.

Current PRs

The top three need a final review but should be good to go 🤞. Can't quite remember about the status of the bottom one, I think most of it's been merged from memory.

What Else

The to do list in the first post is up to date. Probably the next thing to think about is prepend/postpend layouts.

@smithdc1
Copy link
Member Author

Hey all,
A short update as it's been a while since I've commented here.

Prepend/postpend layouts are now merged and the test coverage is also much higher. Now up to the 60% range vs about 30% with the first release.

While the to-do list above is up to date I think the next steps are to:

  • Document CSSContainer. I think this should be useful to help customize styles. Any feedback on the API here would be useful.
  • Tidy up the tests
  • fix flake8

@justinmayer
Copy link
Contributor

Hey @smithdc1! First of all, many thanks for creating such a useful project. I really appreciate all your work on making it easier to use TailwindCSS with Django forms. ✨

As you may have heard, TailwindCSS v2.0 was just released this week, and while I don’t yet fully understand the scope of the new changes, the TailwindCSS folks have provided a v2 upgrade guide.

It’s not clear to me what might be needed for this project to support the changes in the new version. Do you have any thoughts about what might be required?

@smithdc1
Copy link
Member Author

@justinmayer thanks for you kind comments and your pull requests. 🙏

Having read through their docs I don't think too much has changed that impacts this project. We didn't use the additional form module, but now that it's a recommended option it's something we could look at.

I had a quick look this morning and the demo site seemed to work ok but I need to take a much closer look to be more certain. 🕵️

@justinmayer
Copy link
Contributor

@smithdc1: My pleasure. I hope that the flood of PRs wasn't too annoying. I tried to break things up into discrete bits, since as a project maintainer myself I often find smaller, more-digestible submissions to be helpful.

Many thanks for having a look at Tailwind CSS v2. When time affords, I look forward to hearing more about what changes (if any) might be afoot. 👣

@smithdc1
Copy link
Member Author

Hi @justinmayer thanks for all your effort over the weekend. It is so very much appreciated.

We were wondering if you would be interested in joining the crispy-forms org?

@justinmayer
Copy link
Contributor

Hi @smithdc1. You are most welcome. It was my pleasure to contribute to this very useful project.

Thank you for the thoughtful invitation. I would be honored to join the crispy-forms organization. While I've been using Django for over a decade, I've only recently started using Crispy Forms with Tailwind CSS, so I imagine I'll have further suggestions and contributions as that exploration deepens. 💫

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

4 participants