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

Introduce settings page and more options #111

Merged
merged 8 commits into from
Mar 22, 2019
Merged

Introduce settings page and more options #111

merged 8 commits into from
Mar 22, 2019

Conversation

stklcode
Copy link
Contributor

@stklcode stklcode commented Oct 3, 2018

This PR covers multiple issues and improvements. Feel free to edit or add.

Most significant change is the introduction of a new settings page. The widget backview is reduced to the two widget-related options (about top lists). I added a link to jump directly to the new page to not annoy existing users that are used to the old config panel.

Second I introduced three new checkboxes to control the skip_tracking behavior. I added the existing blacklist option to this group alongside with "logged in users", "feed" and "search". This partially covers #103, however the remaining options are not controllable yet. Might be combined with @2ndkauboy's solution to allow total control for pro users.
Especially the user agent filter with the RegEx might be suitable to configure here, but this is more of and advanced feature and not that easy to understand.

With the new page, the role filter (#106) can be integrated smoothly.
PR #72 is somewhat obsoleted or has to be adapted to the new method.

Finally two screenshots of the results:
statify_106_screenshot2
statify_106_screenshot1

The backview of the dashboard widget has been reduced to widget settings
and the complete list of settings aloing with short descriptions have
been moved into a new settinge page.
Added three options to not skip tracking. preview, trackback, 404 and
robots are still hardcoded.
@stklcode stklcode mentioned this pull request Oct 3, 2018
Copy link
Member

@patrickrobrecht patrickrobrecht left a comment

Choose a reason for hiding this comment

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

Looks quite good. I've just added a commit fixing the text domain and changing some of the descriptions. I'm wondering whether the settings title can be put into <label> for the corresponding inputs - add_settings_field doesn't seem to have an option for that. WP seems to do that on the WP settings pages (except for checkboxes).

Maybe the "skip tracking" options could be displayed as the options on the WP "Discussion" settings page.

With an additional argument on the add_settings_field() call the given
titles are wrapped in <label> tags. Removed the duplicated title
attributes in favor of this feature.
@stklcode
Copy link
Contributor Author

stklcode commented Oct 22, 2018

I'm wondering whether the settings title can be put into <label> for the corresponding inputs - add_settings_field doesn't seem to have an option for that.

Sure it does: https://developer.wordpress.org/reference/functions/add_settings_field/

$args
(array) (Optional) Extra arguments used when outputting the field.

  • 'label_for'
    (string) When supplied, the setting title will be wrapped in a element, its for attribute populated with this value.
  • 'class'
    (string) CSS Class to be added to the element when the field is output.

Default value: array()

Didn't know that until 5 minutes ago, but it obviously works, see latest commit 😃

@Zodiac1978 Zodiac1978 added this to the 1.7.0 milestone Oct 23, 2018
For consistency do not rely on core translation for these words and add
the custom textdomain "statify".
Not every user who can see stats should be able to configure every
 setting and probably not even edit the widget itself. Separated the
 permissions required to limit access to new options and potentially
 allow more flexible granting filters.
@stklcode
Copy link
Contributor Author

stklcode commented Oct 26, 2018

The admin menu is yet limited to the manage_options privilega, however the links are shown with edit_dashboard. Bad UX when clicking a link ends up in a permission denied page.

add_options_page(
__( 'Statify', 'statify' ),
__( 'Statify', 'statify' ),
'manage_options',
'statify-settings',
array( __CLASS__, 'create_settings_page' )
);

I separated the 3 possible permissions:

  • See dashboard widget - edit_dashboard or filter statify__user_can_see_stats
  • Edit dashboard widget - edit_dashboard
  • Edit all settings - manage_options

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.

3 participants