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

Generic Settings Pages: Make getSettingPageFilter() public so we can use it in hooks #15576

Merged
merged 1 commit into from
Oct 24, 2019

Conversation

mattwire
Copy link
Contributor

Overview

If you use the generic settings page and then want to load a custom resource (eg. javascript to conditionally hide fields) you can use the buildForm hook but can only match on CRM_Admin_Form_Generic which will match multiple forms. To target an individual "page" we need to know the value of the filter.

Before

Not possible to get the filter value from the form.

After

Can retrieve the filter value from the form and use it to conditionally load resources.

Technical Details

The property _filter is available but private, it can be accessed via the protected method getSettingPageFilter(). We need to check the filter value to work out which settings "page" we are loading.

Comments

@eileenmcnaughton This seems like a simple change, not sure if there is a better way to do this. But being able to match on a specific filter makes the generic settings pages much more useful because they can be individually customised via buildForm hooks.

@civibot
Copy link

civibot bot commented Oct 22, 2019

(Standard links)

@civibot civibot bot added the master label Oct 22, 2019
@eileenmcnaughton
Copy link
Contributor

@mattwire I don't have a problem with this except that maybe you should add a comment as to why it's public so it doesn't later get un-publiced.

Also I imagine you are aware you can add or remove things from the form using the alterSettingsMetaData hook - I can see that you are saying conditional so that's by the by

@eileenmcnaughton
Copy link
Contributor

test this please

@mattwire
Copy link
Contributor Author

@mattwire I don't have a problem with this except that maybe you should add a comment as to why it's public so it doesn't later get un-publiced.

Thanks. I've added a comment. And just used the alterSettingsMetadata hook on another project :-)

@eileenmcnaughton
Copy link
Contributor

@mattwire we have a big picture issue that we don't really define what functions we expect sites to use when interacting with core classes that can be accessed in hooks.

Ideally we would make the right calls about what is public & private & perhaps use interfaces & documentation. We seem to be a long way from there but we should be thinking about it.

@seamuslee001 seamuslee001 merged commit 9634dba into civicrm:master Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants