search

python-discord / django-crispy-bulma

Django application to add 'django-crispy-forms' layout objects for Bulma.io
MIT License
25 stars 9 forks source link

Field template update to Bulma suggested layout #29

Closed andrei-dracea closed 4 years ago

andrei-dracea commented 4 years ago

Added .control wrapper element inside field template to preserve layout from Bulma docs

lemonsaurus commented 4 years ago

Hi @andrei-dracea, thanks for the PR!

Can you describe in more detail what exactly this change does? Maybe show a screenshot or something to illustrate?

Also, when you reformat the entire file in your PR it can be a bit tricky to review since it obscures what you actually changed. It's fine this time since the formatting change seems to be an improvement, but do try to keep that in mind in the future.

andrei-dracea commented 4 years ago

Yes, this change updates the field layout to correspond to bulma layout as you can see here, by adding a .control wrapper on the actual control in the field template. This ensures a number of proper css rules afterwards.

gdude2002 commented 4 years ago

I'm mostly curious whether the change can be expected to break layouts people might be using already, if so, we should document it - but it shouldn't, right?

andrei-dracea commented 4 years ago

Well, it might. Considering the possibility that they might have written some custom css rules within the current structure (for example .field > .input). But I still think it is a good idea to follow Bulma guidelines.

lemonsaurus commented 4 years ago

Yeah, I think that all sounds reasonable, and we should probably get this merged. Because of the custom css rule possibility, I guess we should at least bump the version to 0.2. one might argue that a breaking change should lead to a major version increase, not a minor version increase, but I'm not comfortable calling this v1.0 before it's far, far more done than it currently is. This repo is really still in its infancy.

lemonsaurus commented 4 years ago

@andrei-dracea can you bump the version up to 0.2 in the setup.py in this PR? We need it bumped to be able to do a release via the CI.

andrei-dracea commented 4 years ago

done.

lemonsaurus commented 4 years ago

much appreciated.