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

Add warning for reset button #197

Closed
danyj opened this issue Jan 29, 2015 · 22 comments
Closed

Add warning for reset button #197

danyj opened this issue Jan 29, 2015 · 22 comments

Comments

@danyj
Copy link
Contributor

danyj commented Jan 29, 2015

if reset button in theme options is clicked the values are reset to default and automatically saved.
Can we have info for the user that the button will actually save the values on click,
or change the button text from
"Reset " to "Reset and save"
or just reset options values and let the admin save them again.

I catch my self forgetting for example what the default color value is , by hitting reset I dont expect it to save it automatically but rather just " show me " what defaults are.

@ghost ghost assigned sergiubagrin Jan 29, 2015
@ghost
Copy link

ghost commented Jan 29, 2015

There a 3 solutions:

  1. Change button text to "Reset and save"

  2. On "Reset" to not save the options

  3. Change the success message from

    The options were successfully reset

    to something like

    The options were successfully reset and saved

@danyj
Copy link
Contributor Author

danyj commented Jan 29, 2015

Here is small preview of what I have in our framework. Not saying you should do this but it kinda made sense to me.
http://screencast.com/t/WDS2S6Inq5Gl

So I vote for 2 ,
Do not save on reset , flash some info that options have been reset.

@ghost ghost assigned ghost and unassigned sergiubagrin Jan 29, 2015
@ghost
Copy link

ghost commented Jan 29, 2015

Ok 2.

@danyj
Copy link
Contributor Author

danyj commented Jan 29, 2015

👍

ghost pushed a commit that referenced this issue Jan 29, 2015
@sergiubagrin
Copy link
Contributor

@danyj - Added to "To DO list"

Thanks

@ghost
Copy link

ghost commented Jan 30, 2015

@sergiubagrin Already done. Will be available in the next version.

@ghost
Copy link

ghost commented Jan 31, 2015

v2.1.19 released

@ghost ghost closed this as completed Jan 31, 2015
@danyj
Copy link
Contributor Author

danyj commented Jan 31, 2015

Does not seem like this made it to 2.1.19 , my reset button is still saving the options.
Am I missing something?

@ghost
Copy link

ghost commented Jan 31, 2015

my reset button is still saving the options

Option values are stored in the database in fw_theme_settings_options:{theme-id} wp_option.
You can see that after pressing the Reset button, wp_option value becomes an empty array.

Maybe you can share a screencast for more details? How did you find that options are still saving on reset?

@danyj
Copy link
Contributor Author

danyj commented Jan 31, 2015

Here ,
http://screencast.com/t/xwwvWMYWS
Admin bar is by default enabled,
I disabled the admin bar in frontend and saved the options
Hit reset and admin bar is back.

@ghost
Copy link

ghost commented Jan 31, 2015

After reset the value is "No"=="Don't disable the admin bar". Everything is correct.
Or I am missing something?

@danyj
Copy link
Contributor Author

danyj commented Jan 31, 2015

Yes but it should not auto save it as we discussed previously.
Check the video again , I did not hit save after the reset, it saved it automatically

@ghost
Copy link

ghost commented Jan 31, 2015

@danyj
Copy link
Contributor Author

danyj commented Jan 31, 2015

maybe we have terminology misunderstanding here
load/reset ends up being same as save since I have option default loaded/saved
without me hitting save button.

This is my option output

$styles = fw_get_db_settings_option('get_styles');
echo $styles['gadget'];

echos red in this video
http://screencast.com/t/R05jbvVm

My understand is that reset button click will not affect frontend output unless I click on save button.
But as you can see it does.

@ghost
Copy link

ghost commented Feb 1, 2015

For most developers it's very convenient that the default value is extracted from the settings options when no value is found in the database. That way you don't bother to specify $default_value in function second argument and keep it manually in sync with the default 'value' => '...' from the option array. #11 (comment)

reset ends up being same as save since I have option default loaded without me hitting save button

The only convenience I see (and I thought this is what you requested) is while you developing the theme settings and playing with options default 'value' => ..., you want that value to be reflected instantly in on the Theme Settings page (after refresh). When the option is saved in the database, changing option default 'value' => ... will not be reflected on the page, because the database value will overwrite it.

$styles = fw_get_db_settings_option('get_styles');
echo $styles['gadget'];

echos red in this video
http://screencast.com/t/R05jbvVm

If I understand you correctly, you expect that $styles['gadget'] should be an empty string (or null). You can achieve that by specifying default value as second parameter

echo fw_get_db_settings_option('get_styles/gadget', '');

It works like this:

Unyson, give me settings option value $values['get_styles']['gadget'], if you don't find it in the database, return me '' (empty string).

If you don't specify default value (null), it will try to load it from the settings options array (code).

@danyj
Copy link
Contributor Author

danyj commented Feb 1, 2015

Again misunderstanding.
All I expect is the options not to be saved without me hitting the save button.
Everything else is correct and how it should be.

I dont expect $styles['gadget'] to be empty on reset , I expect it to be as is now but

the frontend should not be changed at all unless I have saved the options and not when I hit reset.

I displayed here what I expect reset button to do
http://screencast.com/t/WDS2S6Inq5Gl
Reset form should be exactly that. Form reset nothing else.

Imagine this
Send email form with clear form button next to save.

You fill it out and hit clear, meaning , I want to clear the form not send it , but in your scenario clear sends the form out first and than clears it.

@ghost
Copy link

ghost commented Feb 1, 2015

I think it's simpler and more intuitive how it works now (with save), the user just hits the Reset button.

Without save, we must show/teach the user that after Reset he needs to press the Save button.
For e.g. a message somewhere "The form values were reset to defaults. Now you can press Save...".
(Also the user has to do 2 actions now).

I understand that you are trying to create the same behavior as in Yjsg settings page and that you have many clients that are familiar with that behavior. But we also have clients that are familiar with the "reset+save" behavior.

Now I don't know: To choose only one behavior (with or without save) or somehow to implement them both.

@danyj
Copy link
Contributor Author

danyj commented Feb 1, 2015

Not sure bud honestly. The "not saving" , gives me the upper hand to see what the default was before it is saved.

If you have any color option set to, let say #e2a140
than you change it to fafafa
you will not know what default was and you have to dig to find it. Reset only will show you what it was before you save.

Or check this
http://screencast.com/t/h7VGpTCXh2

Imagine 50 patterns and 50 images.

User gets theme , changes the theme option and everything is ok.
Now he plays around with background options and want to go back to what it was for that one option. If he hits reset( and reset saves ) , ALL of his options will go back to what they were without him wanting to do that.
But he does not know what the default was, pattern, image, what image was it ?
With only reset ( no saving ) he can see what it was.

But if this is much for you and you think that WP users mind set is not going to welcome this, than don't brake your self, just add a warning that the options are about to be reset. Like avada did.
http://screencast.com/t/33y5hCybXpr

@ghost ghost reopened this Feb 1, 2015
@ghost ghost added the analyze label Feb 1, 2015
@ghost
Copy link

ghost commented Feb 1, 2015

We will do it like avada did http://screencast.com/t/33y5hCybXpr

@ghost ghost added enhancement and removed analyze labels Feb 2, 2015
ghost pushed a commit that referenced this issue Feb 2, 2015
@ghost
Copy link

ghost commented Feb 3, 2015

Added confirm alert in v2.1.20

@ghost ghost closed this as completed Feb 3, 2015
@ghost
Copy link

ghost commented Feb 7, 2015

There is no need to set the values in the database, the defaults are read from the settings options array if nothing is set in the database.

Sorry, that is a bug in the fw_get_db_settings_option() function:

  • fw_print(fw_get_db_settings_option('get_styles')) shows the right default value from the settings options
  • fw_print(fw_get_db_settings_option()) shows the empty array from the database, instead of reading defaults from the settings options

I will fix that soon (in the next version).

@danyj
Copy link
Contributor Author

danyj commented Feb 7, 2015

yes I figured that one just now thus I deleted my post. the actual option is there but the array of options is empty. thnx for replying.

ghost pushed a commit that referenced this issue Feb 17, 2015
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants