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

Always observe Modal changes #1881

Merged
merged 5 commits into from
Oct 4, 2022
Merged

Always observe Modal changes #1881

merged 5 commits into from
Oct 4, 2022

Conversation

mvorisek
Copy link
Member

@mvorisek mvorisek commented Oct 2, 2022

fix #1880

discussion #334 (comment)

marking as BC-break a several settings Modal methods were removed

@mvorisek mvorisek marked this pull request as ready for review October 2, 2022 23:01
// add setting if available.
if (isset($this->options['setting'])) {
foreach ($this->options['setting'] as $key => $value) {
$this->js(true)->modal('setting', $key, $value);
Copy link
Member Author

Choose a reason for hiding this comment

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

@ibelar any idea why the settings were not passed to the Fomantic-UI Modal constructor (initial ->js(true)->modal call) directly?

@mvorisek mvorisek added the RTM label Oct 2, 2022
@mvorisek mvorisek force-pushed the always_observe_changes branch from db8f0b3 to ae438c2 Compare October 3, 2022 15:14
@mvorisek mvorisek merged commit 9256fbd into develop Oct 4, 2022
@mvorisek mvorisek deleted the always_observe_changes branch October 4, 2022 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modal should set observeChanges by default
1 participant