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

Added label display control #41

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Added label display control #41

wants to merge 4 commits into from

Conversation

diopib
Copy link

@diopib diopib commented Nov 18, 2013

added feature to hide label or not.
example: {{ field|bootstrap:False}}

added label display control
added label display control
label display management (updating merge request)
@nadrane
Copy link

nadrane commented Dec 5, 2013

I'm really struggling with this exact same issue right now. In essence, label tags without an value are getting rendered, messing with the alignment of my page.

What are the chances we include this in the official release?

@diopib
Copy link
Author

diopib commented Dec 5, 2013

depends on the repo owner. You can still fork my version for now.

@tzangms
Copy link
Owner

tzangms commented Dec 5, 2013

Hi @zoner14 I will get this pull request merged and release tomorrow.

I just get back from release a new product of my company.

@nadrane
Copy link

nadrane commented Dec 5, 2013

This is actually my first time using Github. I'm shocked how easy this was. Thank you so much! I really didn't want to have to hack together my own solution such that it differed from the standard release.

@tzangms
Copy link
Owner

tzangms commented Dec 5, 2013

@diopib @zoner14

Why not just turn label off by setting it in Form?

Like this

field = forms.CharField(label="")

though ModelForm will be a little bit more complex

@diopib
Copy link
Author

diopib commented Dec 5, 2013

@tzangms
I think you shouldn't have to change basic form data...
Just found it would be cleaner to manage it from the template (better if you have complex templates with many conditional statements etc.) But maybe the argument passing can be improved.

@tzangms
Copy link
Owner

tzangms commented Dec 5, 2013

@diopib

In Django 1.6, it allows you to override labels in ModelForm, like this

class AuthorForm(ModelForm):
    class Meta:
        model = Author
        labels = {
            'field_name: "",
        }

I think it's the way that Django works, settings things up in Form, but not to increase the complexity in template.

if you turn of label in Form/ModelForm, you can use just one line in template.

{{ form|bootstrap }}

but seperate every fields into template, and turn one field's label off.

@diopib
Copy link
Author

diopib commented Dec 5, 2013

@tzangms
I think it all depends on what you want to do, how you want to do it or what django version you're working with. in my case I needed to handle it in the template, that's why I made those changes...
Feel free to accept/reject the pull request (currently using my forked version though).
But thanks for this app, the idea is great and it's very well thought!

@nadrane
Copy link

nadrane commented Dec 5, 2013

So I actually had first tried setting the label equal to null, and this actually does not work. The API still renders the label.

Here is what one of my form fields looks like:

username = forms.CharField(max_length = 30,
                                          label = '',
                                          widget=forms.TextInput(attrs = {'placeholder': 'Username'})
                                         )

and here is how it renders in HTML after calling {{ form|bootstrap}}
hmmmm looks like github filters out my html!

Suffice to say, the label renders like this:< label class="control-label " for="id_username">

@diopib
Copy link
Author

diopib commented Dec 6, 2013

Still thinking that changes have to be made on your app, you shouldn't have to "tweak" a django form to achieve that. You should manage the display on the template, not elsewhere (good practice).
Thanks all.

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

Successfully merging this pull request may close these issues.

3 participants