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

theme_options filter isn't always triggered for exhibit themes #113

Open
michaelhagedon opened this issue Jul 13, 2017 · 4 comments
Open

Comments

@michaelhagedon
Copy link
Contributor

Once I've saved an exhibit's theme configuration, I can no longer access the values via the theme_options filter. Omeka_Form_ThemeConfiguration::init() does call this filter via Theme::getOptions() if nothing has been saved, but Exhibit Builder never calls the filter if theme options have been loaded from the exhibits table.

Steps to reproduce

  1. Implement the theme_options filter in a plugin class like so:
protected $_filters = ['theme_options'];

public function filterThemeOptions($serializedOptions, $args)
{
    echo 'asdf';
    return $serializedOptions;
}
  1. Edit an exhibit, switch to another theme, and configure it.
  2. Notice the 'asdf' at the bottom of the page.
  3. Save the theme configuration form.
  4. Press Configure again.

Expected behavior

'asdf' should still be at the bottom of the page.

Actual behavior

'asdf' is not present.

@zerocrates
Copy link
Member

Doesn't the same behavior happen with the "regular" form once it's been saved?

The theme_options helper isn't really designed for changing what appears in the form, but changing the actual options as they're applied on the public views.

@michaelhagedon
Copy link
Contributor Author

michaelhagedon commented Jul 13, 2017

Doesn't the same behavior happen with the "regular" form once it's been saved?

I had that question, too, but no the default public theme config always calls this filter. It works as expected.

The theme_options helper isn't really designed for changing what appears in the form, but changing the actual options as they're applied on the public views.

Right, I figured that out... what I'm ultimately wanting to do is what you suggest: I have an in-house "branding" plugin that I want to add option elements to all theme config forms from a plugin. That would require adding a hook or filter. Might that be something EB core would merge in if I sent a pull request? Of course, assuming good coding practices.

But this issue was my plan B. I wanted to introduce a hidden field in themes' config.ini that I could then read from plugins. I expected the filter to apply on the config form just cause that's where I was testing. I see what you're saying now... the filter does apply when the exhibit is viewed, so that's good enough for me for Plan B. This bug still represents unexpected behavior, but I'm not sure anyone will care. :-)

Still interested to hear what you think about the pull request above, though. Thanks!

@zerocrates
Copy link
Member

I had that question, too, but no the default public theme config always calls this filter. It works as expected.

Hmm... I had presumed that the core also used the same system for passing in current settings, but I see that it doesn't, accounting for the difference. I don't think we can simply call the theme_options filter manually when getting the Exhibit-configured settings either, because it presents a little bit of a cyclical problem, as Exhibit Builder hooks into that filter itself.

So you're thinking of adding a hook or filter actually designed to alter the form? That sounds like it would be fine as a pull request, though I think it would go toward the core, and not the Exhibit Builder, as it sounds like something you'd change in the core's ThemeConfiguration form.

With what you've got in current Omeka, without adding a new filter or hook, the best option for doing something like what you're describing would probably be to have the settings in the plugin's configuration page, then use the theme_options filter to merge them into the configuration. Of course, if your aim is to just provide consistent options but have the actual settings vary from theme to theme, that doesn't work as well.

@zerocrates
Copy link
Member

Actually on closer examination Exhibit Builder's theme_options filter doesn't get used on the admin side, so it might be feasible to just apply_filters('theme_options' ... in Exhibit Builder's theme form code. The inconsistency you mention isn't too easy to notice because we don't have a whole lot of plugins (besides Exhibit Builder itself) actually using this filter.

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

No branches or pull requests

2 participants