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

Filter refactor #575

Merged
merged 24 commits into from
Feb 24, 2024
Merged

Filter refactor #575

merged 24 commits into from
Feb 24, 2024

Conversation

justinkruit
Copy link
Member

@justinkruit justinkruit commented Sep 15, 2023

Adding a lot of filters to replace the small functions with.
This will make it easier to make changes for other developers.

  • Turned bootscore_sidebar_col_class into a filter (6d7e18a)
  • Turned bootscore_sidebar_toggler_class into a filter (6d7e18a)
  • Turned bootscore_sidebar_offcanvas_class into a filter (6d7e18a)
  • Turned bootscore_container_class into a filter (afb590d c1d8393 d01bd42 1d6c386)
    • Each reference of this filter has an argument included of which page it is. This way you can filter based on that when adding filters to your own code.
      An example is included below.
  • Turned bootscore_main_col_class into a filter (94239fc)
  • Added a filter to the SCSS compiler if the user wants to make changes to $compiler before the compile happens (e903f0d)
  • Added a filter to make Font Awesome toggleable (4ebdd63) (Fixes [Feature Request] Filter to remove fontawesome #566)
  • All the old class functions are being properly handled in deprecated.php (f1d8939)

Examples

Change the container of the header to container-fluid:

function container_class($string, $location) {
  if ($location == 'header') {
    return "container-fluid";
  }
  return $string;
}

add_filter('bootscore/container_class', 'container_class', 10, 2);

Turn off Font Awesome

add_filter('bootscore/load_fontawesome', '__return_false');

@crftwrk
Copy link
Member

crftwrk commented Sep 15, 2023

Cool, I'm in. Outdated scripts we have to put to https://github.com/bootscore/bootscore/blob/main/inc/deprecated.php to keep functionality for overridden templates.

While we are here, do we want include deny direct access and version as described in #367 in this PR? Version should be 5.3.4.

<?php
/**
* Breadcrumb
*
* @package Bootscore
* @version 5.3.3
*/
// Exit if accessed directly
defined( 'ABSPATH' ) || exit;

@crftwrk crftwrk added the documentation Improvements or additions to documentation label Sep 15, 2023
@justinkruit
Copy link
Member Author

The new commit f1d8939 checks if the child theme has the old function included, and if it does, it will use that old return value as the filter. This way the update won't be a breaking change (yet).

@justinkruit
Copy link
Member Author

@crftwrk How far do we want to go with adding filters?
For example, strings like this can also be wrapped in a filter to easily change without the need to copy the whole template file.

<span class="h5 offcanvas-title"><?php esc_html_e('Menu', 'bootscore'); ?></span>

@crftwrk
Copy link
Member

crftwrk commented Sep 15, 2023

Think changing the classes of titles etc. will go a bit too much ;-)

@justinkruit
Copy link
Member Author

Think changing the classes of titles etc. will go a bit too much ;-)

Didn't mean the classes in this case, but the text like "menu". Having such strings replaceable is what makes the filters in woocommerce so "powerful". We could also go that route.

@crftwrk
Copy link
Member

crftwrk commented Sep 15, 2023

Understand, I‘m in. We have to list all of them in the docs.

@justinkruit
Copy link
Member Author

Looks good to me! We could even expand this behavior in #575 and add filters to the ifs to always show them without the need to change the template part.

What do you think of this idea @crftwrk?
It could work like this (rough sketch):


to

if ( is_account_page() && apply_filters('bootscore_wc_actions_hide_account_on_account_page', true)) { 

@crftwrk
Copy link
Member

crftwrk commented Oct 27, 2023

It could work like this (rough sketch)

Yes, this seems to be a good idea 🙏

page.php Outdated
<div id="primary" class="content-area">

<!-- Hook to add something nice -->
<?php bs_after_primary(); ?>

<div class="row">
<div class="<?= bootscore_main_col_class(); ?>">
<div class="<?= apply_filters('bootscore_main_col_class', 'col'); ?>">
Copy link
Member

Choose a reason for hiding this comment

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

This may create issues, because col isn't enough. If main content has for example a large image, col takes the entire width of the container and pushes sidebar down. It has to checked if a sidebar is active or not. See this very first PR #7.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Great 👍

@zsvendo
Copy link

zsvendo commented Nov 17, 2023

If possible, create the positioning of the sidebars or where they should appear. For example, natively switch sides or even not show it on the single-product page

@justinkruit
Copy link
Member Author

Time to rename all the filters, I want them with / instead of _ 😇

@justinkruit justinkruit marked this pull request as ready for review February 23, 2024 13:17
@justinkruit
Copy link
Member Author

I think this version of the PR is ready to be reviewed and merged.

The buttons haven't been done, but for now that would lead to additional clutter in the theme. Maybe we could transform the button into a function to call with all the details in an array? Then the array can be added as a filter in the function itself.

Something like (first argument is just a name for the location and help the filter)

<?= bootscore_button('offcanvas/search', ['text' => '<i class="fa-solid fa-magnifying-glass"></i><span class="visually-hidden-focusable">Search</span>', 'class' => 'btn btn-outline-secondary d-lg-none ms-1 ms-md-2 top-nav-search-md']);

@crftwrk
Copy link
Member

crftwrk commented Feb 23, 2024

Tried

function container_class($string, $location) {
  if ($location == 'header') {
    return "container-fluid";
  }
  return $string;
}

add_filter('bootscore_container_class', 'container_class', 10, 2);

and

add_filter('bootscore_load_fontawesome', '__return_false');

but nothing happens?

@justinkruit
Copy link
Member Author

justinkruit commented Feb 23, 2024

Since the original commit, filters have been renamed. The ones you need are bootscore/container_class and bootscore/load_fontawesome. I've just updated that in the example above.

@crftwrk
Copy link
Member

crftwrk commented Feb 23, 2024

Got it, works fine.

Possible to add multiple locations here?

  if ($location == 'header') {

@justinkruit
Copy link
Member Author

justinkruit commented Feb 23, 2024

Of course. All the locations are set in the places the filter is being called in the template files. So for example here:

<div class="<?= apply_filters('bootscore/container_class', 'container', 'footer'); ?>">

$location will be footer (third argument in the apply_filters function). This has been done as much as possible throughout all the locations filters have been added.

@crftwrk
Copy link
Member

crftwrk commented Feb 23, 2024

No, I mean targeting multiple files in one function like if location header, footer, page-sidebar-left. If so, what is the snippet if i want to target containers only in header and footer?

@justinkruit
Copy link
Member Author

If you want to want to have a fluid container in both the header and footer, it will be like this:

function container_class($string, $location) {
  if ($location == 'header' || $location == 'footer') {
    return "container-fluid";
  }
  return $string;
}

add_filter('bootscore/container_class', 'container_class', 10, 2);

As simple as that 😄

@crftwrk
Copy link
Member

crftwrk commented Feb 23, 2024

Exactly what I'm searching for 👍

@crftwrk
Copy link
Member

crftwrk commented Feb 23, 2024

I love it, but I need more information how to use them in the docs. Can you drop a quick example how to change roughly the main-col breakpoints/width and sidebar breakpoints/width included the toggler plus product cols? This will prevent us from answering issues. ;-)

@justinkruit
Copy link
Member Author

Writing out some examples of the filters, will drop them this evening!

@crftwrk
Copy link
Member

crftwrk commented Feb 23, 2024

Thank you.

Should this function not wrapped in if (!function_exists('bootscore_sidebar_col_class_sidebar')) { to override in child?

function bootscore_main_col_class_sidebar($string) {
if (is_active_sidebar('sidebar-1')) {
// Sidebar is not empty
return "col-md-8 col-lg-9";
}
return $string;
}
add_filter('bootscore/main/col_class', 'bootscore_main_col_class_sidebar');

@justinkruit
Copy link
Member Author

Should this function not wrapped in if (!function_exists('bootscore_sidebar_col_class_sidebar')) { to override in child?

In that case they can just add another filter 😄

@justinkruit
Copy link
Member Author

justinkruit commented Feb 23, 2024

Here are a handful of examples that I could think of. A lot of things are possible with filters, and developers can make changes for niche situations in their own theme.

Disable FontAwesome
add_filter('bootscore/load_fontawesome', '__return_false');
Set a new logo
function set_bootstrap_logo($logo, $size) {
  if ($size === 'small') {
    return get_stylesheet_directory_uri() . '/path/to/small-logo.svg';
  }

  return get_stylesheet_directory_uri() . '/path/to/normal-logo.svg';
}

add_filter('bootscore/logo', 'set_bootstrap_logo', 10, 2);
Set the offcanvas title
function set_offcanvas_title($title) {
  return 'My Menu';
}

add_filter('bootscore/offcanvas/navbar/title', 'set_offcanvas_title');
Set the content spacer on an archive page of the post type "product"
function set_content_spacer($spacer) {
  if (is_post_type_archive('product')) {
    return 'mt-5';
  }

  return $spacer;
}

add_filter('bootscore/content/spacer_class', 'set_content_spacer');
Change the container class in the header and footer
function container_class($string, $location) {
  if ($location == 'header' || $location == 'footer') {
    return "container-fluid";
  }
  return $string;
}

add_filter('bootscore/container_class', 'container_class', 10, 2);
Make changes in inc/blocks/block-widget-archives.php
function change_block_widget_archives($block_content, $block) {
  // do something with $block_content
  return $block_content;
}

add_filter('bootscore/block/archives/content', 'change_block_widget_archives', 10, 2);
Disable the SCSS compiler
add_filter('bootscore/scss/disable_compiler', '__return_true');
Change the col size when the sidebar is active (other sizes than default)
function change_content_col_size($col_size) {
  if (is_active_sidebar('sidebar-1')) {
    return "col-md-6 col-lg-7";
  }

  return $col_size;
}

add_filter('bootscore/main/col_class', 'change_content_col_size');

function change_sidebar_col_size($col_size) {
  return "col-md-6 col-lg-5 order-first order-md-last";
}

add_filter('bootscore/sidebar/col_class', 'change_sidebar_col_size');
Remove order-first order-md-last from sidebar
function remove_order_first_last($col_class) {
  return str_replace('order-first order-md-last', '', $col_class);
}

add_filter('bootscore/sidebar/col_class', 'remove_order_first_last');

Copy link
Member

@crftwrk crftwrk left a comment

Choose a reason for hiding this comment

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

Ok, got it and this is damn hot, thank you! As i see, there is no single recipe how to use them, but offers many possibilities. For the docs we should start with simple examples how to change the containers and main-col breakpoints and adding more examples from time to time.

@crftwrk crftwrk merged commit 6896a76 into main Feb 24, 2024
@crftwrk
Copy link
Member

crftwrk commented Apr 3, 2024

@justinkruit I am on the docs part for the filters right now, but I can't get this to work?

function change_content_col_size($col_size) {
  if (is_active_sidebar('sidebar-1')) {
    return "col-md-6 col-lg-7";
  }

  return $col_size;
}

add_filter('bootscore/main/col_class', 'change_content_col_size');

function change_sidebar_col_size($col_size) {
  return "col-md-6 col-lg-5 order-first order-md-last";
}

add_filter('bootscore/sidebar/col_class', 'change_sidebar_col_size')

@justinkruit
Copy link
Member Author

Let me try that in a couple of hours when I'm home. It does seem correct.

@justinkruit
Copy link
Member Author

@crftwrk Found the issue, it's because we renamed the hooks (again).

add_filter('bootscore/class/col/main', 'change_content_col_size');
add_filter('bootscore/class/col/sidebar', 'change_sidebar_col_size');

Use these and it should work.
I also highly recommend using PhpStorm, as that gives suggestions on the existing hook names.
image

@crftwrk
Copy link
Member

crftwrk commented Apr 3, 2024

Thanks, it works for the sidebar but not for the main content?

@justinkruit
Copy link
Member Author

Where do you place the code to test?

@crftwrk
Copy link
Member

crftwrk commented Apr 3, 2024

https://dev.bootscore.me/category/clerkship/ Sidebar is below the main-col

function change_content_col_size($col_size) {
  if (is_active_sidebar('sidebar-1')) {
    return "col-lg-6 ";
  }

  return $col_size;
}

add_filter('bootscore/class/col/main', 'change_content_col_size');

function change_sidebar_col_size($col_size) {
  return "col-lg-6 order-first order-lg-last";
}

add_filter('bootscore/class/col/sidebar', 'change_sidebar_col_size');

@justinkruit
Copy link
Member Author

I mean, where in the code did you place it? File name and line

@crftwrk
Copy link
Member

crftwrk commented Apr 3, 2024

Of course functions.php in child.

is it something here https://github.com/bootscore/bootscore/blob/main/inc/columns.php?

@justinkruit
Copy link
Member Author

Of course functions.php in child.

So this is the reason. This happens because of the "priority" system in the filters. To quote the documentation:

Used to specify the order in which the functions associated with a particular filter are executed.
Lower numbers correspond with earlier execution, and functions with the same priority are executed in the order in which they were added to the filter.

In this case, the child is loaded before the parent. So this line is executed after the code in the child, overriding it. To make it work, add a priority of 11 to our new filter (10 being the default). Now it will be executed after the code of the main theme, effectively overriding it.

function change_content_col_size($col_size) {
  if (is_active_sidebar('sidebar-1')) {
    return "col-lg-6 ";
  }

  return $col_size;
}

add_filter('bootscore/class/col/main', 'change_content_col_size', 11);

Fun fact: because the code in columns.php has already executed, the $col_size in the function above isn't col like by default, but already col-lg-9. This because it already got changed here.

@crftwrk
Copy link
Member

crftwrk commented Apr 4, 2024

Got it, thanks. Sometimes things are so simple... ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking documentation Improvements or additions to documentation feature
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Feature Request] Filter to remove fontawesome
3 participants