Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Form] Added an AbstractChoiceLoader to simplify implementations and handle global optimizations#34550

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

HeahDude
Copy link
Contributor

@HeahDudeHeahDude commentedNov 23, 2019
edited
Loading

QA
Branch?master
Bug fix?no
New feature?yes
Deprecations?no
Tickets~
LicenseMIT
Doc PR~

Taking over#33218 (taking over#30983)

TheChoiceLoaderInterface is not easy to understand/implement, while its goal is simple, lazy load an array of choices.
What may seem complicated is the how/what to optimize loading, we need to deal with a$value callback (which refers to thechoice_value option allowing to transform a model choice to a unique string value that can be displayed/submitted).

We have now enough implementations in core to justify the need of an abstraction and provide a better DX in the process.

Before this PR we needed to implement 3 methods to create a loader:

  • loadChoiceList(?callable $value): ChoiceListInterface
  • loadChoicesForValues(array $values, ?callable $value): array
  • loadValuesForChoices(array $choices, ?callable $value): array
    and handle optimization, in each.

Now we only need to implement:

  • loadChoices(): iterable

and optionnally:

  • doLoadChoicesForValues(array $values, ?callable $value): array
    if more optimization is needed to only load submitted values, (i.e the core intl loader prevents loading values that are the same as choices, and the doctrine one performs aWHERE id IN ($ids) query).

@HeahDudeHeahDudeforce-pushed theform/abstract-choice-loader branch 2 times, most recently from49d04e5 tof1ab485CompareNovember 24, 2019 17:05
@nicolas-grekasnicolas-grekas added this to thenext milestoneNov 24, 2019
@HeahDude
Copy link
ContributorAuthor

Appveyor failures unrelated. This one is ready on my side.

@HeahDudeHeahDudeforce-pushed theform/abstract-choice-loader branch 2 times, most recently fromef26f16 tod9379adCompareNovember 25, 2019 18:57
@javiereguiluz
Copy link
Member

javiereguiluz commentedNov 27, 2019
edited
Loading

Hi Jules! This looks "complicated" because different people have attempted to solve this in multiple different pull requests. So, could you please update your pull request description with a link to the issue/PR which explains how this feature will behave for end users?

If not such a link exists, please update the PR description to briefly explain what does this do, how does it work, show a simple example, etc.

Thanks!

HeahDude reacted with thumbs up emoji

@HeahDude
Copy link
ContributorAuthor

Thank you for your comment@javiereguiluz, I've updated the description.

javiereguiluz reacted with heart emoji

@HeahDudeHeahDudeforce-pushed theform/abstract-choice-loader branch 2 times, most recently fromed731e1 to961582bCompareDecember 2, 2019 11:57
@HeahDudeHeahDudeforce-pushed theform/abstract-choice-loader branch 4 times, most recently fromb6cad00 to528f9f3CompareDecember 8, 2019 13:30
@HeahDudeHeahDudeforce-pushed theform/abstract-choice-loader branch from528f9f3 to354816bCompareDecember 14, 2019 17:17
@HeahDudeHeahDudeforce-pushed theform/abstract-choice-loader branch 2 times, most recently fromd10b2d2 to0f4c3b0CompareFebruary 9, 2020 13:43
@HeahDudeHeahDudeforce-pushed theform/abstract-choice-loader branch from0f4c3b0 to1394df2CompareFebruary 9, 2020 23:49
@fabpot
Copy link
Member

Thank you@HeahDude.

HeahDude and yceruto reacted with hooray emoji

fabpot added a commit that referenced this pull requestFeb 12, 2020
…mentations and handle global optimizations (HeahDude)This PR was merged into the 5.1-dev branch.Discussion----------[Form] Added an AbstractChoiceLoader to simplify implementations and handle global optimizations| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       | ~| License       | MIT| Doc PR        | ~<!--Replace this notice by a short README for your feature/bugfix. This will help peopleunderstand your PR and can be used as a start for the documentation.Additionally (seehttps://symfony.com/roadmap): - Always add tests and ensure they pass. - Never break backward compatibility (seehttps://symfony.com/bc). - Bug fixes must be submitted against the lowest maintained branch where they apply   (lowest branches are regularly merged to upper ones so they get the fixes too.) - Features and deprecations must be submitted against branch master.-->Taking over#33218 (taking over#30983)The `ChoiceLoaderInterface` is not easy to understand/implement, while its goal is simple, lazy load an array of choices.What may seem complicated is the how/what to optimize loading, we need to deal with a `$value` callback (which refers to the `choice_value` option allowing to transform a model choice to a unique string value that can be displayed/submitted).We have now enough implementations in core to justify the need of an abstraction and provide a better DX in the process.Before this PR we needed to implement 3 methods to create a loader: - `loadChoiceList(?callable $value): ChoiceListInterface` - `loadChoicesForValues(array $values, ?callable $value): array` - `loadValuesForChoices(array $choices, ?callable $value): array`and handle optimization, in each.Now we only need to implement: - `loadChoices():  iterable`and optionnally: - `doLoadChoicesForValues(array $values, ?callable $value): array`if more optimization is needed to only load submitted values, (i.e the core intl loader prevents loading values that are the same as choices, and the doctrine one performs a `WHERE id IN ($ids)` query).Commits-------1394df2 [Form] Added an AbstractChoiceLoader to simplify implementations and handle global optimizations
@fabpotfabpot merged commit1394df2 intosymfony:masterFeb 12, 2020
@HeahDudeHeahDude deleted the form/abstract-choice-loader branchFebruary 12, 2020 21:06
fabpot added a commit that referenced this pull requestMar 16, 2020
…eahDude)This PR was merged into the 5.1-dev branch.Discussion----------[Form] Added a "choice_filter" option to ChoiceType| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| Deprecations? | yes| Tickets       |Fix#32657| License       | MIT| Doc PR        |symfony/symfony-docs#13223Finally opening this PR for a very old branch, based on both#34550 (merged) and#30994 (merged).~Until#30994 is merged, this PR should better be reviewed by commits. Thanks!~Commits-------ed2c312 [Form] Added a "choice_filter" option to ChoiceType
@nicolas-grekasnicolas-grekas modified the milestones:next,5.1May 4, 2020
@fabpotfabpot mentioned this pull requestMay 5, 2020
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@stofstofstof left review comments

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@xabbuhxabbuhAwaiting requested review from xabbuh

Assignees
No one assigned
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.

6 participants
@HeahDude@javiereguiluz@fabpot@nicolas-grekas@stof@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp