Myzwer / foothillschurch

Bootcamp II is a wordpress theme (as well as an inside joke) designed to suit the needs of foothillschurch.com. It makes use of webpack, Babel, Sass, Tailwind, Browsersync, PostCSS, ESLint, Stylelint, Prettier and more. It is meant for that site, but if you can use it by all means go for it.
1 stars 1 forks source link

Form Success code cleanup #41

Closed rain2o closed 1 month ago

rain2o commented 1 month ago

Just a few small cleanup notes.

$counter isn't used as far as I can tell, can be removed. https://github.com/Myzwer/foothillschurch/blob/effc5bbf81497ae99f0561ab7849412fe9272bfd/form-success.php#L31

else isn't needed unless you actually have something to put in the else block. https://github.com/Myzwer/foothillschurch/blob/effc5bbf81497ae99f0561ab7849412fe9272bfd/form-success.php#L81


This is just a suggestion of an alternative way of handling flexible content layouts like this. https://github.com/Myzwer/foothillschurch/blob/effc5bbf81497ae99f0561ab7849412fe9272bfd/form-success.php#L35-L78

You could use the name of the layout as a template partial name. For example, you could create components/calls-to-action/primary_cta.php, etc... Then, in your code call it like this

while (have_rows('call_to_action')) : the_row();
    $layout = get_row_layout();
    get_template_part("components/calls-to-action/$layout");
endwhile;

It's just an option if you want to cleanup this file a bit and keep those layouts separate as individual components.

rain2o commented 1 month ago

Here's another location where you could use the layout name as part of the template partial filename to simplify your implementation (the last suggestion in this issue).

https://github.com/Myzwer/foothillschurch/blob/effc5bbf81497ae99f0561ab7849412fe9272bfd/location.php#L126-L154

rain2o commented 1 month ago

Another place you could simplify the implementation of layout template partial

https://github.com/Myzwer/foothillschurch/blob/effc5bbf81497ae99f0561ab7849412fe9272bfd/outreach.php#L23-L40

rain2o commented 1 month ago

I've noticed several places where you have the following logic https://github.com/Myzwer/foothillschurch/blob/effc5bbf81497ae99f0561ab7849412fe9272bfd/resources-bank.php#L19-L44

In addition to the suggestion I originally posted in this issue about using the value of get_row_layout() as part of the partial path directly, another suggestion I have is to move this logic into its own template part. Maybe something like header_select.php that just contains this logic. That way you're not repeating it all a bunch.