search

WPBuddy / largo

A WordPress framework for news websites. Finely-crafted by INN and expertly-honed and maintained by the technology team at WP Buddy.
http://largo.wpbuddy.co
GNU General Public License v2.0
171 stars 83 forks source link

Posts Per Page on Landing Page Editor Doesn't Reflect "All" Option #1908

Closed joshdarby closed 3 years ago

joshdarby commented 3 years ago

Expected Behavior

When saving the Posts per page option on the landing page builder, saving the "all" option should reflect as such on page refresh.

Actual Behavior

Saving "all" doesn't actually work and just defaults back to 5. Might be because the actual amount saved is 0? Might be something else.

https://github.com/WPBuddy/largo/blob/825dce582300231d4723cad32e497b4824df5bc0/inc/wp-taxonomy-landing/functions/cftl-admin.php#L433-L445

Screen Shot 2021-05-13 at 8 28 13 PM

Steps to Reproduce the Problem

  1. Go to landing page builder
  2. Set "posts per page" to "all"
  3. Reload page and notice wrong value
joshdarby commented 3 years ago

The solution to this would be to change this line https://github.com/WPBuddy/largo/blob/825dce582300231d4723cad32e497b4824df5bc0/inc/wp-taxonomy-landing/functions/cftl-admin.php#L610 to sanitize_text_field.

The reason it's not working now is because when all is selected, it attempts to save the value as all, which isn't an intval so naturally it fails

benlk commented 3 years ago

Is all a valid value for the per_page query parameter? It may be better to refactor this to a list of value => label, like 999 => __( 'All', 'largo' ),