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

Tweak: maybe add the same capability check (manage_settings) also for js as menu icon #72

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

geckod22
Copy link

Proposed changes

the module does this two calls:

	add_action( 'admin_init', array( $this, 'register_assets' ) );
	add_action( 'admin_bar_menu', array( $this, 'newfold_help_center' ), 11 );

the menu icon is showed if:
current_user_can( 'manage_options' )
&& is_admin()
&& $this->container->get( 'capabilities' )->get( 'canAccessHelpCenter' )

while the css and js are loaded if:
file_exists( $asset_file )
&& $this->container->get( 'capabilities' )->get( 'canAccessHelpCenter' )

so what happens is that if I have a contributor role and access the admin dashboard, I do not see the help center icon in topbar as I do not have the manage_options but the css and js are still loaded - I think we should avoid this but I am not sure the js is loaded to make the functions available to a third part thing

Type of Change

Production

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Dependency update
  • Refactoring / housekeeping (changes to files not directly related to functionality)

If you login as contributor for example, you will not see the help centr menu icon in topbar but the scripts are still loaded

imagen

Development

  • Tests
  • Dependency update
  • Environment update / refactoring
  • Documentation Update

Video

Checklist

  • I have read the CONTRIBUTING doc
  • I have viewed my change in a web-browser
  • Linting and tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

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.

1 participant