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

SelectField choice refactoring #739

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

azmeuk
Copy link
Member

@azmeuk azmeuk commented Apr 28, 2022

Fixes #692. Related to #381.

SelectField choice argument can take a list of 3 items tuples, the last one being a dict that will be rendered as <option> HTML parameter.

@azmeuk azmeuk force-pushed the select-option-customization branch 3 times, most recently from 2c7d45f to 3a1d060 Compare April 28, 2022 19:55
@azmeuk azmeuk added the enhancement New feature, or existing feature improvement label May 3, 2022
@kontur
Copy link

kontur commented Nov 7, 2022

This would be very useful 👍

@azmeuk
Copy link
Member Author

azmeuk commented Jan 13, 2023

@davidism does this sound safe to you?

@azmeuk azmeuk force-pushed the select-option-customization branch from 27515f4 to 99c0a64 Compare January 13, 2023 14:31
@davidism
Copy link
Member

davidism commented Jan 13, 2023

Seems good to me. Instead of passing longer tuples, you might want to consider a SelectChoice dataclass with the fields, which might make typing easier later.

@azmeuk
Copy link
Member Author

azmeuk commented Jan 13, 2023

Do you mean we should break the compatibility and expect the choices parameter to be a collection of SelectChoice instead of a collection of tuples?
I suppose this would make the code more explicit and easier to read though.

@davidism
Copy link
Member

Don't add a 3-tuple data type. Instead, only support this new data in the new dataclass. If you get a string, or a 2-tuple, convert it to the dataclass.

Since choices can be set after init, I'd do this conversion in the render step for now (and maybe in init as well). I'd probably keep the string shortcut, but show a deprecation warning for 2-tuples like "Passing (value, label) is deprecated, pass SelectChoice(...) instead.".

@azmeuk azmeuk force-pushed the select-option-customization branch 4 times, most recently from 5148434 to 416842a Compare July 22, 2023 19:20
@azmeuk
Copy link
Member Author

azmeuk commented Jul 22, 2023

Sorry for the noise with the force pushes, that should be good now.

I refactored the SelectField by adding a Choice dataclass. I deprecated passing choices by tuples or dicts. The Choice dataclass carry the optgroup and render_kw.
This was a good idea @davidism I did not realized of much this would simplify the SelectField behavior 👍

I could use another review 🙏

@azmeuk azmeuk changed the title <option> HTML parameters can be set in SelectField choices argument SelectField choice refactoring Jul 22, 2023
@azmeuk azmeuk force-pushed the select-option-customization branch from 416842a to cc721a9 Compare July 28, 2023 19:18
Comment on lines 52 to 57
warnings.warn(
"Passing SelectField choices as tuples is deprecated and will be "
"removed in wtforms 3.3. Please use Choice instead.",
DeprecationWarning,
stacklevel=2,
)

Choose a reason for hiding this comment

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

Please reconsider this. It will be a significant amount of busy-work for larger projects to adapt to this, without any real benefit. Just generating the Choice instance from the tuple is cheap and doesn't seem to have any drawbacks.

image

Yeah... I really do not want to spend time updating those almost 50 places in my app.

@azmeuk azmeuk force-pushed the select-option-customization branch 2 times, most recently from f9337a5 to 5bc3653 Compare October 5, 2023 09:41
@azmeuk azmeuk force-pushed the select-option-customization branch from 5bc3653 to fada111 Compare October 5, 2023 09:41
@azmeuk azmeuk merged commit a6f44f0 into pallets-eco:master Oct 5, 2023
7 checks passed
@azmeuk azmeuk deleted the select-option-customization branch October 5, 2023 09:45
azmeuk added a commit that referenced this pull request Oct 11, 2023
@aminalaee aminalaee mentioned this pull request Oct 11, 2023
@coleifer
Copy link

This breaks a few libraries, including wtf-peewee which passes two-tuples.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature, or existing feature improvement
Development

Successfully merging this pull request may close these issues.

SelectField - can choices tuples be expanded to allow for tooltips
5 participants