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

Replace database table configuration with Settings API? #70

Open
kaittodesk opened this issue Nov 18, 2020 · 9 comments
Open

Replace database table configuration with Settings API? #70

kaittodesk opened this issue Nov 18, 2020 · 9 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@kaittodesk
Copy link
Member

I am thinking of replacing custom database table based configuration options storing with Wordpress' Settings API, maybe we can utilize it to provide a better plugin experience to the user.

@kaittodesk kaittodesk added enhancement New feature or request question Further information is requested labels Nov 18, 2020
@ghost
Copy link

ghost commented Jan 12, 2021

The database storing is handled by Options API. Settings API is for generating admin forms & pages, Settings API implements Options API but is not limited to it.

@kaittodesk
Copy link
Member Author

I am not 100% certain Settings API can be used as we would like. From the information I have gathered Settings API can be used to add settings fields "Settings" menu item in WordPress' admin panel. I am not sure about adding subpages to "Settings" menu item, because register_setting states about $option_group:

Default allowed option key names include 'general', 'discussion', 'media', 'reading', 'writing', 'misc', 'options', and 'privacy'.

Using the subpage option wouldn't be ideal either, because it can make finding Smaily settings harder.

@tomabel, have you taken an in-depth look of the Settings API (or tried it out)? Would it be possible to keep the menu item placement as it is, but switch forms to Settings API?

If Settings API isn't as flexible as I was hoping, then I think the only option would be to use only Options API.

@ghost
Copy link

ghost commented Jan 21, 2021

Using the subpage option wouldn't be ideal either, because it can make finding Smaily settings harder.

It can still be used with the admin_menu hook, so it appears on the left sidebar.

@kaittodesk
Copy link
Member Author

OK, it is good to know that the menu position can remain the same.

But what about register_setting states about $option_group? Can the list of allowed option key names can be extended?

@ghost
Copy link

ghost commented Jan 29, 2021

But what about register_setting states about $option_group? Can the list of allowed option key names can be extended?

It's possible to specify the option group and option name as smailyforwp_api_option, specifying a different option group would pop a notice/error in my environment.

On a different note, I don't think Settings API is worth it. I can see it being helpful for themes and plugins which have a lot of options that only need sanitization (for example title, name). Our plugin has only a few input fields, the bulk of them for verifying API credentials, of which users need ample feedback on (wrong subdomain, credentials valid).
We would also need to disable auto-loading options. The one way to do that is to run update_option with the autoload parameter set to false. This however would void the benefit of having WP handle the option management itself, as we would be changing options.

However, I think we should implement any benefits of Settings API, like visual consistency and security measures; and use our own implementation of saving/getting data via Options API

@kaittodesk
Copy link
Member Author

Yeah, I was also questioning if Settings API would provide any real value to our use case or just make implementing desired user experience more difficult than it should be.

However, I think we should implement any benefits of Settings API, like visual consistency and security measures; and use our own implementation of saving/getting data via Options API

Can you please elaborate on this a bit or provide a code example (or a pattern) on how the implementation would look like?

@ghost
Copy link

ghost commented Feb 1, 2021

smaily_admin_save() {
	validateOp();
	switch ( $form_data['op'] ) {
		case 'validateApiKey':
			$api_credentials = validateAndSanitize( $form_data );
			$request = makeRequest( $api_credentials );
			if ($request) {
				update_option( 'smailyforwp_api_option', $api_credentials, 'no' );
			}
function smaily_admin_render() {
	$template = new Smaily_Plugin_Template( 'html/admin/page.php' );

	// Load configuration data.
	$data       = get_option('smailyforwp_api_option');
	$template->assign( (array) $data );

@kaittodesk
Copy link
Member Author

Options API is straight forward and easy to follow, but I am more interested in this part:

[...] implement any benefits of Settings API, like visual consistency and security measures; [...]

How were you planning to implement visual consistency and security measures?

@ghost
Copy link

ghost commented Feb 1, 2021

Visually, the API credentials section uses styling which WordPress itself uses. But the section after entering credentials doesn't label user input fields and the second section doesn't use table headers and rows to keep things aligned.
visual_consistency

For a comparison: WordPress' settings page
wp_options_page

I think a checkbox with query parameters can be used in place of jQuery Tabs, which is currently used to control if the basic or advanced form is used. The <ul class="tabs"> element's style can be changed by other plugins that use jQuery tabs & enqueue their own styles on every page in admin.

Had these points in mind under visual consistency. I thought the form needed a nonce, but looking at the source of admin-ajax.php, it runs is_user_logged_in() so no qualms on that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

1 participant