Uneditable trim invisible from select

[smithdc1]

This work was done some time ago but a PR was never raised (that I can see). I've rebased the changes and made a tweak to maintain python 2 support. Fixes #817

[codecov-io]

Codecov Report

Merging #934 into master will increase coverage by 0.23%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #934      +/-   ##
=========================================
+ Coverage   95.17%   95.4%   +0.23%     
=========================================
  Files          24      24              
  Lines        2776    2788      +12     
  Branches      251     251              
=========================================
+ Hits         2642    2660      +18     
+ Misses         90      86       -4     
+ Partials       44      42       -2

Impacted Files	Coverage Δ
crispy_forms/tests/test_form_helper.py	97.37% <100%> (+0.03%)	⬆️
crispy_forms/bootstrap.py	91.39% <100%> (+1.23%)	⬆️
crispy_forms/tests/forms.py	100% <100%> (ø)	⬆️
crispy_forms/layout.py	97.34% <0%> (+1.06%)	⬆️
crispy_forms/utils.py	93.68% <0%> (+2.1%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f38a7df...5b458d1. Read the comment docs.

[carltongibson]

Do you think we need docs for the new option, and a small test case (so we catch any regressions later)?

But good work picking this up! 👍

[smithdc1]

Hi @carltongibson. Thanks for your comments.

Tests, yes. I'll take a look at that.

As for docs, this got me thinking. The solution that was proposed added a new helper, and so yes we should have docs for that. But I've now got some questions!

• it currently defaults to false, should this by true? I suspect that's what most people will expect.
• is there a use case to make want an uneditable field to be editable? Should we just implement this with out the helper option?

@bryan-brancotte appreciate your thoughts to.

[bryan-brancotte]

Hi @smithdc1

Maybe our approach to keep a select is not the good idea, as mentioned in #817 (comment) with boostrap2 it was only a span, so not all the option, maybe we should go back to this behavior and use https://getbootstrap.com/docs/4.0/components/forms/#readonly ?

Pros:

• Do not adding a new option in the helper
• Do not need documentation
• Only render the current value, not the others

Cons:

• If a crispy user create field as read-only and then want to enable it in JS it is not possible

In conclusion : either user readonly of BS4 or just disabled plus not-triming

[smithdc1]

Hi @bryan-brancotte
I didn't quite understand your proposal. Are you suggesting that we either go back to bs2 style or that no change should be made?

If so I suggest the latter, this is especially true if problem be solved by a method in a form class (similar to #935 ) ?

In the meantime I've had another look at this and writing some tests and docs helped me to understand this issue in more detail. Below is my thought process.

I've added three fields to the test project being

• standard select
• disabled select (which generates the options in the HTML cost),
• disable with trim option = True

See here for generated HTML. The fields are just above the submit buttons.

When setting the option to True the field disappears all together rather than outputting a span. I think this is because I need to pass the data to the form for it to match with options?? (sorry, run out of knowledge, again :-)). So question is, does this even work as it is? If it does can we write an appropriate test?

Having written the docs, I don't think it is too intrusive. By default the behaviour is unchanged (no trimming) and so it allows people to enable the disabled field through JS. The trimming function is done through an argument on the uneditable field rather than another addition to the helper. This makes more sense with the longer term project of splitting out the template packs.

[bryan-brancotte]

Hi @smithdc1
I have mixed feeling about this issue, mostly because the initial need is for me wrong : asking crispy to add option to help not exposing the complet list of user is for me note the good approach, I think one should instead in the init of the form trim the queryset for the field associated to User. When trimming in the init the need of a trimming in the field never arise.

For the bs2 approach, if we want to let dev trim the queryset, I proposed https://getbootstrap.com/docs/4.0/components/forms/#readonly as it looks nice, but don't know the amount of work to make it work. Indeed the span is not printed so I guess it is, at least, not working out of the box. So maybe just forget my proposition and keep the trimmed select ;) ?

By default the behaviour is unchanged (no trimming)
👍

The trimming function is done through an argument on the uneditable field
👍