leforestier / yattag

Python library to generate HTML or XML in a readable, concise and pythonic way.
333 stars 31 forks source link

updated input list to include 'file' and 'tel' types #63

Closed StephonBrown closed 4 years ago

StephonBrown commented 4 years ago

I saw an issue about not including a file type for Simple_Input, so I added it and 'tel' to the list as options. I also added an additional test to make sure it functioned correctly.

leforestier commented 4 years ago

Hi, thanks for the pull request.

when you create a document using the Doc constructor, you can give it two optional parameters, defaults and errors. What makes the input method useful, compared to just creating form fields using the tag method, is just that the input method looks into the defaults and errors parameters of the document to add a possible default value and/or an error message to the field. Default values can be used as initial values in a form, but what they are most commonly used for is to display original values that the user submitted, in case there's an error in the form they submitted, so that they can correct their incorrect inputs, and so they don't lose their correct inputs. The "file" type of input is a bit particular because the value that the user submits is a file, not a string, and what the application sees is a stream handler. That's why I hadn't added that type of input in the possible types for the input method: adding a default value makes no sense here. And if the programmer isn't careful, they might end up trying to insert a stream handler inside the generated document without realizing it.

That said, I thought about it again thanks to your pull request. I think this could still be useful to have error messages for this type of field. But to avoid the problem I just mentioned, we're going to have to modify the SimpleInput class so that it doesn't insert any value in the field in the case the field is of type "file". Do you want to take care of this?

leforestier commented 4 years ago

Actually, it would probably be better to raise an exception if there's a default value for a field of type file.

StephonBrown commented 4 years ago

Thanks for the lesson. I had not considered the stream handlers when generating a form for a file input field (which I should have). I can make this change, but two questions:

leforestier commented 4 years ago

About "tel": before you raised the issue I didn't know that "tel" was a possible html5 input field . This is definitely something that should be added.

About "file", I think you understood that correctly. If the input field is of type "file", and if the name of the field appears in the defaults dictionary, an exception should be raised. Something like "default value for HTML input of type file is not supported". Or tell me if you can think of a better message.

StephonBrown commented 4 years ago

Kept the 'tel' type and implemented a test and DocError raise if default values are placed for the 'file' input type.

leforestier commented 4 years ago

Thanks Stephon, I think that was a nice addition to the project.

leforestier commented 4 years ago

Your change is now available on Pypi (yattag version 1.14.0).