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

Fixes #14 -- Added Float Field #42

Merged
merged 4 commits into from
May 26, 2021
Merged

Fixes #14 -- Added Float Field #42

merged 4 commits into from
May 26, 2021

Conversation

smithdc1
Copy link
Member

@millerthegorilla -- here's a draft implementation for floating fields. Do you want to take a look?

The test shows how this would be used. Is this the right way of doing this? 🤔

Copy link

@millerthegorilla millerthegorilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, that is working for me. It is a shame that placeholder text can't be overridden.
I tried the following in the field file.

    def __init__(self, *args, **kwargs):
        breakpoint()
        super().__init__(*args, **kwargs)
        self.attrs["placeholder"] = kwargs.get('placeholder', self.fields[0])

in the form I define the field as

FloatingField('email', css_class="mb3", placeholder="Email Here")

This sets the self.attrs["placeholder"] to 'Email Here. Unfortunately, the whole mechanism of bootstrap5's floating fields is to present the floating label as the placeholder text, until input. Ah well. Just to note, really, that you can set the placeholder text to anything you want in the field, and it will be set to the value of 'label'.
So, worth mentioning in the docs, I guess, that floating fields don't have placeholder text.
EDIT
I have just tried defining a label for the field in the form meta class and it doesn't show up. However, it does when I define the following in my form init,

         self.fields['email'].label = "Email Here!"
        self.helper.layout = Layout(.....

It also works in the case where I define a field with a label in the form itself ie:

     username = forms.CharField(label="Your username here!")

please keep in mind that I am testing against bootstrap4 crispy errors_and_help_text code. As soon as I get a mo, probably tomorrow, I will make a new branch for my code base, and try migrating to crispy bootstrap 5, and will test this again then.

@smithdc1 smithdc1 marked this pull request as ready for review May 5, 2021 20:35
@smithdc1
Copy link
Member Author

smithdc1 commented May 5, 2021

@millerthegorilla -- anything else to add here?

If not I'll write up a small documentation note -- probably to the readme -- and look to merge this.

@millerthegorilla
Copy link

I will take a look first thing tomorrow, but I still haven't got round to switching my project to crispy bs5 yet.

@eriktelepovsky
Copy link

Perfect! Looking forward to merge this.

@smithdc1
Copy link
Member Author

@eriktelepovsky Have you given this a run? Do you think the way this would be used in forms is right?

@millerthegorilla
Copy link

Hi, sorry I got snowed under. I just checked this and it looks really good, and works well, at first glance. Looking forward to it being merged.

@millerthegorilla
Copy link

millerthegorilla commented May 19, 2021

Just one thing I notice straight away, probably me not getting it straight, but there are a few floating fields in a form, that have the following html below them -
<small id="hint_id_display_name" class="form-text text-muted">display name</small>
I guess this is from help_text_and_errors, but I don't have help text defined for those floating fields that have this hint text, and not all the floating fields are displaying a hint.
I have added help_text to the model field that is displayed and cannot see the help text in the form using 'floatingfield'. However, I've been away from django for a month, and am just warming up to it again. I will return first thing tomorrow, and take a closer look at where I might be going wrong.

@millerthegorilla
Copy link

Ok, I had some spurious help_text defined. The only issue that I can see is that when I have help_text defined in the model rather than the form, it is not showing in the rendered html.

@smithdc1
Copy link
Member Author

Ok, let's merge this. Thanks all for your input.

The only issue that I can see is that when I have help_text defined in the model rather than the form, it is not showing in the rendered html.

This sounds like a separate issue, I'm not sure without taking a closer look

@smithdc1 smithdc1 force-pushed the float branch 2 times, most recently from e7aadd8 to a50d0f6 Compare May 21, 2021 07:23
@millerthegorilla
Copy link

I haven't tested selects, but it also works fine with textareas which is nice.

@millerthegorilla
Copy link

@smithdc1 is there any reason this can't be merged? I am hoping to roll out my crispy forms bootstrap5 site, with the funky new floatfields!

@smithdc1
Copy link
Member Author

Ok, I added additional tests for failing fields, text area and selects. All seem to look ok to me. 🙂

@smithdc1 smithdc1 merged commit 63428aa into main May 26, 2021
@smithdc1 smithdc1 added this to the Next Release milestone May 27, 2021
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