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

Theme settings max input vars #3327

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ViorelEremia
Copy link
Contributor

No description provided.

@ViorelEremia
Copy link
Contributor Author

It's a dirty way but I don't see any solution to keep compatibility with form extended from the class form.
@danyj, @andreiglingeanu can you test

@danyj
Copy link
Contributor

danyj commented Apr 25, 2018

on this

@ViorelEremia
Copy link
Contributor Author

First of all test some options with big deep multi levels arrays. I tried some addable popup in adabalbe popup with switch and again adable popup and is fine.

@danyj
Copy link
Contributor

danyj commented Apr 25, 2018

when you say test,
what should I look for , missing options in multi dimensional or?

@danyj
Copy link
Contributor

danyj commented Apr 25, 2018

ah I see, form data is 1 now , ok , checking everything

@danyj
Copy link
Contributor

danyj commented Apr 25, 2018

I think more of us should test this , I do have some large options and multies but someone might have a combo that can cause some issues. Il open a issue calling for more devs

@ViorelEremia
Copy link
Contributor Author

I mean if all options are saved. Decrease max_input_vars in php.ini and in theme settings add some options types with big deep arrays for example, such as addable popup in adable popup

@danyj
Copy link
Contributor

danyj commented Apr 25, 2018

looks ok on my end ,
only issue I had is that I was doing silent save on export data option type via

		var $form = $('.fw-settings-form');
		var data = $form.serialize();

                    $.ajax({
                        type: 'post',
                        dataType: 'json',
                        url: ajaxurl,
                        data: data

                    }).....

and now it should be

	var $form = $('.fw-settings-form');

	var $submitButton = $form.find(':submit:focus');
	var data = $form.serialize() + ( $submitButton.length ? '&' + $submitButton.attr( 'name' ) + '=' + $submitButton.attr( 'value' ) : '' );

	if ( $form.attr( 'data-fw-form-id' ) === 'fw-settings-form:theme-settings' ) {
		data = {'fw_theme_settings_form': data};
	}

@ViorelEremia , when do you think this will be out in stable ?

@ViorelEremia
Copy link
Contributor Author

I thought this week but I think it's too late or maybe the prerelease on Friday and update Wednesday.

@danyj
Copy link
Contributor

danyj commented Apr 25, 2018

checked resets also , looks ok , I would still love to see other test as well since this is a major change

@danyj
Copy link
Contributor

danyj commented May 4, 2018

@ViorelEremia Don't release this jet please, I had to revert back
to solve this #3348 (comment)

the addable popup returns null ,

I think there is issue with commas and or quotes
in that pupup there is textarea with several class names

http://prntscr.com/jdsfdm

input[type="text"],input[type="number"],input[type="search"],input[type="password"],input[type="email"],input[type="tel"],input[type="url"],input[type="datetime"],input[type="date"],input[type="datetime-local"],input[type="month"],input[type="week"],input[type="time"],select,textarea,.select2-container .select2-choice,#bbp_topic_content,#bbp_reply_content,.thz-site-html .select2-drop-active,.thz-site-html .select2-selection--single,.thz-site-html .woocommerce-page input.select2-search__field,.thz-site-html .select2-dropdown,fieldset

not sure if that is causing the null to be returned from the pupup ,

so with this current change same popup with 2 elements
http://prntscr.com/jdsfy9

returns this

https://prnt.sc/jdrjai

and it should be this

http://prntscr.com/jdserv

@ViorelEremia
Copy link
Contributor Author

ViorelEremia commented May 7, 2018

@danyj now should work try my last commit

@danyj
Copy link
Contributor

danyj commented May 7, 2018

will do

@danyj
Copy link
Contributor

danyj commented May 8, 2018

@ViorelEremia Still not ok ,

now all my typography options are null ,

http://prntscr.com/jf8cs0

http://prntscr.com/jf8cvx

@danyj
Copy link
Contributor

danyj commented May 8, 2018

and the thing is that everything is present in form data

body_font%5D%5Bvalue_data%5D=%7B%22family%22%3A%22Nunito%22%2C%22weight%22%3A%22regular%22%2C%22subset%22%3A%22latin%22%2C%22size%22%3A%2216%22%2C%22line-height%22%3A%221.8%22%2C%22color%22%3A%22%23757575%22%2C%22css%22%3A%22font-family%3A'Nunito'%2Csans-serif%3Bfont-size%3A16px%3Bfont-weight%3Anormal%3Bline-height%3A1.8%3Bcolor%3A%23757575%3B%22%2C%22google_font_link%22%3A%22Nunito%3Aregular%26subset%3Dlatin%22%2C%22style%22%3A%22default%22%2C%22spacing%22%3A%22%22%2C%22transform%22%3A%22default%22%2C%22align%22%3A%22default%22%2C%22hovered%22%3A%22%22%2C%22text-shadow%22%3A%5B%5D%7D&fw_options%5Bsitelc%5D%5Bvalue_data%5D=%7B%22lc%22%3A%22color_2%22%2C%22lh%22%3A%22color_1%22%7D&fw_options%5Bheadings_font%5D%5Bvalue_data%5D=%7B%22family%22%3A%22Nunito%22%2C%22weight%22%3A%22700%22%2C%22subset%22%3A%22latin%22%2C%22css%22%3A%22font-family%3A'Nunito

on reset it works , once you save, data is sent, but output is null

@danyj
Copy link
Contributor

danyj commented May 8, 2018

this is the working typo print

array
(
    [family] => 'Nunito'
    [weight] => 'regular'
    [subset] => 'latin'
    [size] => '16'
    [line-height] => '1.8'
    [color] => '#757575'
    [css] => 'font-family:'Nunito',sans-serif;font-size:16px;font-weight:normal;line-height:1.8;color:#757575;'
    [google_font_link] => 'Nunito:regular&subset=latin'
)

@ViorelEremia
Copy link
Contributor Author

in your string, the last character should be ' like: amily%3A'Nunito' not as is now amily%3A'Nunito
Or you showed only a part of the string

@danyj
Copy link
Contributor

danyj commented May 8, 2018

it was missing I think, tried to pull it out of long string

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.

None yet

2 participants