search

methnen / m-chart-highcharts-library

Adds Highcharts library to M Chart.
Other
7 stars 1 forks source link

cache theme lookup and add ability to filter them #26

Closed nkryptic closed 2 years ago

nkryptic commented 2 years ago

The basis for this PR was that we needed to remove all theme choices besides those that were "approved". Without something like this, I had to hook into the m_chart_settings_template and write a "wrapper" template that captured the output of rendering the actual settings template. Then, I removed the unwanted theme options with preg_replace. Not very pretty.

What this PR does: Caches the themes found during the get_themes call and adds a filter hook on the themes, providing the ability to add/remove programmatically.

nkryptic commented 2 years ago

I'll make the recommended changes. As to the cache refreshing, I've only ever used the built-in object cache which refreshes on each page load, so thanks for the tip. I think refreshing the cache on the settings page is a must. Do you think it acceptable to not refresh the cache on the new/edit chart page?

nkryptic commented 2 years ago

I've made the changes. Here's what would be added to the Action and filter hooks page in the m-chart wiki: m-chart-wiki-flexible-themes-pr.md

methnen commented 2 years ago

A lot of WP specialty hosts use memchache or other persistent back ends for the object cache and those can hang around a LONG time. So that was my concern. I was thinking about this on my commute today and it occurs to me that maybe another for the cache refresh would also be on the chart edit page. That way if someone ads a new theme and then hits the chart edit page their new theme will show. It also means the user won't ever have to actually think about it. At any point where they'd expect recently added themes to be available they'd be available.

nkryptic commented 2 years ago

I've merged the refresh for the new/edit chart page and added the relevant hook to the wiki page.