Skip to content

[Autocomplete] implement preload option #539

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

Merged
merged 1 commit into from
Nov 28, 2022
Merged

[Autocomplete] implement preload option #539

merged 1 commit into from
Nov 28, 2022

Conversation

seb-jean
Copy link
Contributor

@seb-jean seb-jean commented Nov 9, 2022

Q A
Bug fix? no
New feature? yes
Tickets N/A
License MIT

Added preload option to control the preload.
Default value is false

@seb-jean seb-jean changed the title [Autocomplete] implement preload option [Autocomplete] implement preload option Nov 9, 2022
Copy link
Member

@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

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

This is very cool - thank you! Minor comments

get preload() {
if (this.preloadValue == 'false') return false;
if (this.preloadValue == 'true') return true;
return this.preloadValue;
Copy link
Member

Choose a reason for hiding this comment

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

Can you add some { to the if statements to standardize with the coding style? And also an empty break before the return statement. thx :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the if statements.
I don't see where you want me to put the break.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I meant an empty line break. You did it perfectly 👍

}

return $value;
});
Copy link
Member

Choose a reason for hiding this comment

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

Would it work if we removed this and, instead, above, did:

$values['preload'] = json_encode($options['preload']);

Copy link
Contributor Author

@seb-jean seb-jean Nov 9, 2022

Choose a reason for hiding this comment

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

For boolean values it works, but not for strings

autocomplete

@weaverryan
Copy link
Member

Thanks for this nice PR @seb-jean!

@weaverryan weaverryan merged commit 5f11181 into symfony:2.x Nov 28, 2022
weaverryan added a commit that referenced this pull request Nov 29, 2022
…t (weaverryan)

This PR was merged into the 2.x branch.

Discussion
----------

[Autocomplete] Setting preload back to "focus" as the default

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| Tickets       | None
| License       | MIT

Hi!

In 2.6.0, #539 accidentally changed the `preload` behavior from `focus` to `false`. The result is that, in 2.5.0, clicking on an autocomplete field will instantly make an Ajax call to load options. But in 2.6.0, clicking on it does nothing, until you type a few characters.

This changes back to the old default, which I like better, and also (maybe more importantly) is the previous behavior.

Cheers!

Commits
-------

04cc08f Setting preload back to "focus" as the default
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.

2 participants