WordPress.org

Make WordPress Themes

Opened 3 years ago

Closed 3 years ago

#20550 closed theme (live)

THEME: Pinnacle - 1.2.0

Reported by: britner Owned by: grapplerulrich
Priority: new theme Keywords: theme-pinnacle
Cc: ben@…, grapplerulrich, greenshady, emiluzelac

Description

Pinnacle - 1.0.0

Pinnacle is a bold theme with versatile options and multiple styles. This theme is loaded with features and tools that allow full creativity to be released into a unique site. Built with a modern flat design, its fully responsive layout make for easy navigation on mobile/tablet displays. Pinnacle is perfect for any kind of business, online store, portfolio, or personal site. It is fully compatible with woocommerce and gives you a unique layout for an ecommerce site. Pinnacle was built and designed by <a href="http://kadencethemes.com/">Kadence Themes</a>.

Theme URL - http://kadencethemes.com/product/pinnacle-free-theme/
Author URL - http://www.kadencethemes.com/

SVN - https://themes.svn.wordpress.org/pinnacle/1.0.0
ZIP - https://wordpress.org/themes/download/pinnacle.1.0.0.zip?nostats=1

History:

Ticket Summary Status Resolution Owner
#20550 THEME: Pinnacle - 1.2.0 closed live grapplerulrich

(this ticket)

#23411 THEME: Pinnacle - 1.2.1 closed live emiluzelac
#23921 THEME: Pinnacle – 1.2.2 closed live jcastaneda
#24107 THEME: Pinnacle – 1.2.3 closed live jcastaneda
#24289 THEME: Pinnacle – 1.2.5 closed live jacenty3590
#24308 THEME: Pinnacle – 1.2.6 closed live jacenty3590
#24626 THEME: Pinnacle – 1.2.7 closed live karmatosed
#25383 THEME: Pinnaclele – 1.2.8 closed not-approved emiluzelac
#25442 THEME: Pinnacle – 1.2.8 closed live poena
#26484 THEME: Pinnacle – 1.3.0 closed live jcastaneda
#27201 THEME: Pinnacle – 1.3.1 closed live jcastaneda
#28491 THEME: Pinnacle – 1.3.2 closed live karmatosed
#29065 THEME: Pinnacle – 1.3.3 closed live Otto42
#29415 THEME: Pinnacle – 1.3.4 closed live emiluzelac
#29562 THEME: Pinnacle – 1.3.5 closed live Otto42
#30275 THEME: Pinnacle – 1.3.6 closed live jcastaneda
#31947 THEME: Pinnacle – 1.3.7 closed live themetracbot
#33856 THEME: Pinnacle – 1.3.8 closed live themetracbot
#33981 THEME: Pinnacle – 1.3.9 closed live themetracbot
#34678 THEME: Pinnacle – 1.4.0 closed live themetracbot
#34793 THEME: Pinnacle – 1.4.1 closed live themetracbot
#34839 THEME: Pinnacle – 1.4.2 closed live themetracbot
#35151 THEME: Pinnacle – 1.4.3 closed live themetracbot
#35258 THEME: Pinnacle – 1.4.4 closed live themetracbot
#36876 THEME: Pinnacle – 1.4.5 closed live themetracbot
#37644 THEME: Pinnacle – 1.4.6 closed live themetracbot
#41193 THEME: Pinnacle – 1.4.7 closed live themetracbot
#41321 THEME: Pinnacle – 1.4.8 closed live themetracbot
#41666 THEME: Pinnacle – 1.5.0 closed live themetracbot
#41684 THEME: Pinnacle – 1.5.1 closed live themetracbot
#41795 THEME: Pinnacle – 1.5.2 closed live themetracbot
#41889 THEME: Pinnacle – 1.5.3 closed live Otto42
#41978 THEME: Pinnacle – 1.5.4 closed live themetracbot
#42208 THEME: Pinnacle – 1.5.5 closed live themetracbot
#43384 THEME: Pinnacle – 1.5.6 closed live themetracbot
#44286 THEME: Pinnacle – 1.5.7 closed live themetracbot
#44483 THEME: Pinnacle – 1.5.8 closed live themetracbot
#49080 THEME: Pinnacle – 1.5.9 closed live themetracbot


https://themes.svn.wordpress.org/pinnacle/1.0.0/screenshot.png


Change History (91)

#1 @themetracbot
3 years ago

  • Summary changed from THEME: Pinnacle - 1.0.0 to THEME: Pinnacle - 1.0.1

Pinnacle - 1.0.1

Pinnacle is a bold theme with versatile options and multiple styles. This theme is loaded with features and tools that allow full creativity to be released into a unique site. Built with a modern flat design, its fully responsive layout make for easy navigation on mobile/tablet displays. Pinnacle is perfect for any kind of business, online store, portfolio, or personal site. It is fully compatible with woocommerce and gives you a unique layout for an ecommerce site. Pinnacle was built and designed by <a href="http://kadencethemes.com/">Kadence Themes</a>.

Theme URL - http://kadencethemes.com/product/pinnacle-free-theme/
Author URL - http://www.kadencethemes.com/

SVN - https://themes.svn.wordpress.org/pinnacle/1.0.1
ZIP - https://wordpress.org/themes/download/pinnacle.1.0.1.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=/pinnacle/1.0.0&new_path=/pinnacle/1.0.1

History:

Ticket Summary Status Resolution Owner
#20550 THEME: Pinnacle - 1.2.0 closed live grapplerulrich

(this ticket)

#23411 THEME: Pinnacle - 1.2.1 closed live emiluzelac
#23921 THEME: Pinnacle – 1.2.2 closed live jcastaneda
#24107 THEME: Pinnacle – 1.2.3 closed live jcastaneda
#24289 THEME: Pinnacle – 1.2.5 closed live jacenty3590
#24308 THEME: Pinnacle – 1.2.6 closed live jacenty3590
#24626 THEME: Pinnacle – 1.2.7 closed live karmatosed
#25383 THEME: Pinnaclele – 1.2.8 closed not-approved emiluzelac
#25442 THEME: Pinnacle – 1.2.8 closed live poena
#26484 THEME: Pinnacle – 1.3.0 closed live jcastaneda
#27201 THEME: Pinnacle – 1.3.1 closed live jcastaneda
#28491 THEME: Pinnacle – 1.3.2 closed live karmatosed
#29065 THEME: Pinnacle – 1.3.3 closed live Otto42
#29415 THEME: Pinnacle – 1.3.4 closed live emiluzelac
#29562 THEME: Pinnacle – 1.3.5 closed live Otto42
#30275 THEME: Pinnacle – 1.3.6 closed live jcastaneda
#31947 THEME: Pinnacle – 1.3.7 closed live themetracbot
#33856 THEME: Pinnacle – 1.3.8 closed live themetracbot
#33981 THEME: Pinnacle – 1.3.9 closed live themetracbot
#34678 THEME: Pinnacle – 1.4.0 closed live themetracbot
#34793 THEME: Pinnacle – 1.4.1 closed live themetracbot
#34839 THEME: Pinnacle – 1.4.2 closed live themetracbot
#35151 THEME: Pinnacle – 1.4.3 closed live themetracbot
#35258 THEME: Pinnacle – 1.4.4 closed live themetracbot
#36876 THEME: Pinnacle – 1.4.5 closed live themetracbot
#37644 THEME: Pinnacle – 1.4.6 closed live themetracbot
#41193 THEME: Pinnacle – 1.4.7 closed live themetracbot
#41321 THEME: Pinnacle – 1.4.8 closed live themetracbot
#41666 THEME: Pinnacle – 1.5.0 closed live themetracbot
#41684 THEME: Pinnacle – 1.5.1 closed live themetracbot
#41795 THEME: Pinnacle – 1.5.2 closed live themetracbot
#41889 THEME: Pinnacle – 1.5.3 closed live Otto42
#41978 THEME: Pinnacle – 1.5.4 closed live themetracbot
#42208 THEME: Pinnacle – 1.5.5 closed live themetracbot
#43384 THEME: Pinnacle – 1.5.6 closed live themetracbot
#44286 THEME: Pinnacle – 1.5.7 closed live themetracbot
#44483 THEME: Pinnacle – 1.5.8 closed live themetracbot
#49080 THEME: Pinnacle – 1.5.9 closed live themetracbot


https://themes.svn.wordpress.org/pinnacle/1.0.1/screenshot.png

#2 @themetracbot
3 years ago

  • Summary changed from THEME: Pinnacle - 1.0.1 to THEME: Pinnacle - 1.0.2

Pinnacle - 1.0.2

Pinnacle is a bold theme with versatile options and multiple styles. This theme is loaded with features and tools that allow full creativity to be released into a unique site. Built with a modern flat design, its fully responsive layout make for easy navigation on mobile/tablet displays. Pinnacle is perfect for any kind of business, online store, portfolio, or personal site. It is fully compatible with woocommerce and gives you a unique layout for an ecommerce site. Pinnacle was built and designed by <a href="http://kadencethemes.com/">Kadence Themes</a>.

Theme URL - http://kadencethemes.com/product/pinnacle-free-theme/
Author URL - http://www.kadencethemes.com/

SVN - https://themes.svn.wordpress.org/pinnacle/1.0.2
ZIP - https://wordpress.org/themes/download/pinnacle.1.0.2.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=/pinnacle/1.0.1&new_path=/pinnacle/1.0.2

History:

Ticket Summary Status Resolution Owner
#20550 THEME: Pinnacle - 1.2.0 closed live grapplerulrich

(this ticket)

#23411 THEME: Pinnacle - 1.2.1 closed live emiluzelac
#23921 THEME: Pinnacle – 1.2.2 closed live jcastaneda
#24107 THEME: Pinnacle – 1.2.3 closed live jcastaneda
#24289 THEME: Pinnacle – 1.2.5 closed live jacenty3590
#24308 THEME: Pinnacle – 1.2.6 closed live jacenty3590
#24626 THEME: Pinnacle – 1.2.7 closed live karmatosed
#25383 THEME: Pinnaclele – 1.2.8 closed not-approved emiluzelac
#25442 THEME: Pinnacle – 1.2.8 closed live poena
#26484 THEME: Pinnacle – 1.3.0 closed live jcastaneda
#27201 THEME: Pinnacle – 1.3.1 closed live jcastaneda
#28491 THEME: Pinnacle – 1.3.2 closed live karmatosed
#29065 THEME: Pinnacle – 1.3.3 closed live Otto42
#29415 THEME: Pinnacle – 1.3.4 closed live emiluzelac
#29562 THEME: Pinnacle – 1.3.5 closed live Otto42
#30275 THEME: Pinnacle – 1.3.6 closed live jcastaneda
#31947 THEME: Pinnacle – 1.3.7 closed live themetracbot
#33856 THEME: Pinnacle – 1.3.8 closed live themetracbot
#33981 THEME: Pinnacle – 1.3.9 closed live themetracbot
#34678 THEME: Pinnacle – 1.4.0 closed live themetracbot
#34793 THEME: Pinnacle – 1.4.1 closed live themetracbot
#34839 THEME: Pinnacle – 1.4.2 closed live themetracbot
#35151 THEME: Pinnacle – 1.4.3 closed live themetracbot
#35258 THEME: Pinnacle – 1.4.4 closed live themetracbot
#36876 THEME: Pinnacle – 1.4.5 closed live themetracbot
#37644 THEME: Pinnacle – 1.4.6 closed live themetracbot
#41193 THEME: Pinnacle – 1.4.7 closed live themetracbot
#41321 THEME: Pinnacle – 1.4.8 closed live themetracbot
#41666 THEME: Pinnacle – 1.5.0 closed live themetracbot
#41684 THEME: Pinnacle – 1.5.1 closed live themetracbot
#41795 THEME: Pinnacle – 1.5.2 closed live themetracbot
#41889 THEME: Pinnacle – 1.5.3 closed live Otto42
#41978 THEME: Pinnacle – 1.5.4 closed live themetracbot
#42208 THEME: Pinnacle – 1.5.5 closed live themetracbot
#43384 THEME: Pinnacle – 1.5.6 closed live themetracbot
#44286 THEME: Pinnacle – 1.5.7 closed live themetracbot
#44483 THEME: Pinnacle – 1.5.8 closed live themetracbot
#49080 THEME: Pinnacle – 1.5.9 closed live themetracbot


https://themes.svn.wordpress.org/pinnacle/1.0.2/screenshot.png

#3 @emiluzelac
3 years ago

  • Owner set to Julien Klepatch
  • Status changed from new to reviewing

#4 @britner
3 years ago

Hey @Julien Klepatch
Any update? Anything you need clarified?

Thanks,

Ben Ritner

#5 @Julien Klepatch
3 years ago

Hi Ben,
I am working on your review no worries. I will send you my comments tonight.
Thanks,
Julien

#6 @grapplerulrich
3 years ago

  • Cc grapplerulrich added

I am adding myself as a mentor to this ticket.

#7 @Julien Klepatch
3 years ago

Hi Ben, please find below my comments:

#Theme activation
=> When activating the theme, a notice is displayed:
"This theme comes packaged with the following premium plugin: Virtue/Pinnacle Toolkit. enjoy:)"
Is this a GPL plugin, and what is the purpose of this plugin?

#/lang
default.mo and default.po should be removed and default.pot should be renamed to pinnacle.pot
(highly encouraged but not required)

#readme.txt
=>I couldnt find 'Custom Metaboxes and Fields for Wordpress' in yourTheme
=>Masonry already included in core, you must load the core version
=>MobileDetect cannot be used, you must use is_mobile() function of core
=>Aqua Resizer cannot be used, WordPress has its own image resizer function (see WP_Image_Editor )

#templates/content.php
=>line61 $image not escaped. it should be: esc_attr( $image );
In general, all variables in attributes must be escaped

#functions.php
=>Line 5 in functions.php is incorrect. You are defining the folder languages but have the folder lang.
=>All functions should hook into actions and filters and not run when the file is loaded.

#Core Header Functionality
=>Your theme is lacking the support for the built-in header functionality. You must either remove your custom implementation or support the core implementation of the functionality:
{
add_theme_support( "custom-header", $args )
}

#Custom functions in public namespace must be prepended with a prefix (usually the slug of your theme)
=>lib/utils.php, from line 91 functions name are not prefixed. You need to add a prefix
=>lib/config.php, @ 340 you also need to add a prefix to the function name

#Theme options must be stored with the permission 'edit_theme_options'
=> themeoptions/redux/framework.php @ line 451

#Templates must follow WordPress Architecture. A base.php file cannot be used.
All headers are included via get_header()
All footers are included via get_footer()
All comments templates are included via comments_template()
All sidebars are included using get_sidebar()
All template part files are included via get_template_part()
Any search form markup is included via get_search_form()
Any login form markup is included via wp_login_form()

#Notice on theme activation
Undefined index: blog_title in \wp-content\themes\pinnacle\templates\home\blog-home.php on line 3

#Displaying problems
=>Long site title breaks out of their box, preventing to select below content
=>Some characters do not display properly (encoding problem ?)
=>When menu has a lot of items, page title overlaps with lower row of menu

#Other issues
=> I am still reviewing 3 other items. I will tell you as soon as I have clarified these 3 items.

Thanks,

Julien

#8 @Julien Klepatch
3 years ago

Hi Ben,

Please find below 2 more comments:

# Theme Options - Shop & Product Settings
=>Your theme offers some options related to the woocommerce plugin. It will be better if these options are hidden per default and shown only when woocommerce plugin is activated
(recommended only)

# Tracking in Redux
=> You commented out the code that does tracking on Redux. That's good but not enough. You need to remove this part of the code altogether.

Thanks,

Julien

#9 @britner
3 years ago

Hey,

First a big thanks for your time!

I'm going to number the comments in reply so I can better keep up with your input.

#Theme activation

  1. The virtue / pinnacle toolkit its a plugin on wordpress.org, see here: https://wordpress.org/plugins/virtue-toolkit/

and it's for "Custom Portfolio and Shortcode functionality for Virtue and Pinnacle WordPress Theme"

#/lang

  1. I don't mind changing the .pot file to pinnacle.

#readme.txt

  1. It's in pinnacle/lib/cmb/
  1. I am loading the core version of masonry. Is it a problem to note that it's used in the readme?
  1. MobileDetect can determine if it's a tablet or not, where as is_mobile can't (as far as I know). I don't see why you wouldn't be allowed to use a script that does something the core doesn't do?
  1. The Aqua Resizer uses all core wordpress functions. Meaning it is using "WP_Image_Editor" if I removed the name and you looked at the code you would see that it's using WP_Image_Editor. Plus this has already been discussed and approved by an admin for my other theme virtue.

#templates/content.php

  1. Not a problem I can add that.

#functions.php

  1. lang, yeah missed that, thanks will update.
  1. I can work out a way to load the text domain through filter, it's there like it is because of how the redux framework loads, I'll think up a clever way to do this.

#Core Header Functionality

  1. I don't understand this one. This theme doesn't add a custom header like a banner, you can set a background image to the titleclass but it's not the same as the way a custom header works. And using the custom header or background is not a required thing anyway it's a recommend.

#Custom functions in public namespace

  1. I can add the prefix.
  1. I can add the prefix.

#Theme options

  1. I can add 'edit_theme_options'

#Templates must follow WordPress Architecture.

  1. " A base.php file cannot be used" - I was not aware of this. I also haven't seen in the guidelines can you point it out? That is going to take some work to remove :(

#Notice on theme activation

  1. Thanks, yeah missed that, I'll fix.

#Displaying problems

  1. In the appearance > theme options > Logo option.. the user can define the font size and "Logo Container Width". Meaning if they have a super long title they can adjust the settings to make that work for the header height that they also set in the theme options.
  1. Can you give me an example? what characters?
  1. This can also be changes in the theme options by making the logo width smaller. I can just add an overflow hidden but that would be confusing to a user so I left so people can see what the issue is.

# Theme Options - Shop & Product Settings

  1. I'll look into it, might make a future update.

# Theme Options - Shop & Product Settings

  1. Well just fyi the tracking code is in the file tracking.php which isn't included in the theme, the call for that script is commented out and removing it isn't a problem but for the sake of keeping note of things why does it need to be removed?

Well again thanks for your time, I'll update in the next day with a new version.

Ben

#10 @themetracbot
3 years ago

  • Summary changed from THEME: Pinnacle - 1.0.2 to THEME: Pinnacle - 1.0.3

Pinnacle - 1.0.3

Pinnacle is a bold theme with versatile options and multiple styles. This theme is loaded with features and tools that allow full creativity to be released into a unique site. Built with a modern flat design, its fully responsive layout make for easy navigation on mobile/tablet displays. Pinnacle is perfect for any kind of business, online store, portfolio, or personal site. It is fully compatible with woocommerce and gives you a unique layout for an ecommerce site. Pinnacle was built and designed by <a href="http://kadencethemes.com/">Kadence Themes</a>.

Theme URL - http://kadencethemes.com/product/pinnacle-free-theme/
Author URL - http://www.kadencethemes.com/

SVN - https://themes.svn.wordpress.org/pinnacle/1.0.3
ZIP - https://wordpress.org/themes/download/pinnacle.1.0.3.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=/pinnacle/1.0.2&new_path=/pinnacle/1.0.3

History:

Ticket Summary Status Resolution Owner
#20550 THEME: Pinnacle - 1.2.0 closed live grapplerulrich

(this ticket)

#23411 THEME: Pinnacle - 1.2.1 closed live emiluzelac
#23921 THEME: Pinnacle – 1.2.2 closed live jcastaneda
#24107 THEME: Pinnacle – 1.2.3 closed live jcastaneda
#24289 THEME: Pinnacle – 1.2.5 closed live jacenty3590
#24308 THEME: Pinnacle – 1.2.6 closed live jacenty3590
#24626 THEME: Pinnacle – 1.2.7 closed live karmatosed
#25383 THEME: Pinnaclele – 1.2.8 closed not-approved emiluzelac
#25442 THEME: Pinnacle – 1.2.8 closed live poena
#26484 THEME: Pinnacle – 1.3.0 closed live jcastaneda
#27201 THEME: Pinnacle – 1.3.1 closed live jcastaneda
#28491 THEME: Pinnacle – 1.3.2 closed live karmatosed
#29065 THEME: Pinnacle – 1.3.3 closed live Otto42
#29415 THEME: Pinnacle – 1.3.4 closed live emiluzelac
#29562 THEME: Pinnacle – 1.3.5 closed live Otto42
#30275 THEME: Pinnacle – 1.3.6 closed live jcastaneda
#31947 THEME: Pinnacle – 1.3.7 closed live themetracbot
#33856 THEME: Pinnacle – 1.3.8 closed live themetracbot
#33981 THEME: Pinnacle – 1.3.9 closed live themetracbot
#34678 THEME: Pinnacle – 1.4.0 closed live themetracbot
#34793 THEME: Pinnacle – 1.4.1 closed live themetracbot
#34839 THEME: Pinnacle – 1.4.2 closed live themetracbot
#35151 THEME: Pinnacle – 1.4.3 closed live themetracbot
#35258 THEME: Pinnacle – 1.4.4 closed live themetracbot
#36876 THEME: Pinnacle – 1.4.5 closed live themetracbot
#37644 THEME: Pinnacle – 1.4.6 closed live themetracbot
#41193 THEME: Pinnacle – 1.4.7 closed live themetracbot
#41321 THEME: Pinnacle – 1.4.8 closed live themetracbot
#41666 THEME: Pinnacle – 1.5.0 closed live themetracbot
#41684 THEME: Pinnacle – 1.5.1 closed live themetracbot
#41795 THEME: Pinnacle – 1.5.2 closed live themetracbot
#41889 THEME: Pinnacle – 1.5.3 closed live Otto42
#41978 THEME: Pinnacle – 1.5.4 closed live themetracbot
#42208 THEME: Pinnacle – 1.5.5 closed live themetracbot
#43384 THEME: Pinnacle – 1.5.6 closed live themetracbot
#44286 THEME: Pinnacle – 1.5.7 closed live themetracbot
#44483 THEME: Pinnacle – 1.5.8 closed live themetracbot
#49080 THEME: Pinnacle – 1.5.9 closed live themetracbot


https://themes.svn.wordpress.org/pinnacle/1.0.3/screenshot.png

#11 @Julien Klepatch
3 years ago

Ben,

Thanks for your updates.

#Theme activation

  1. OK

#/lang

  1. OK

#readme.txt

  1. OK
  2. OK
  3. OK
  4. OK

#templates/content.php

  1. previous problem fixed BUT I also found out that some href attributes were not escaped with esc_url() in other files (ex: templates/entry-meta.php @line2). href attributes must be escaped in every file.

#functions.php

  1. OK
  2. OK

#Core Header Functionality

  1. You are right, custom header is not required UNLESS the theme implements its own custom header, which I believe your theme does. Hold on at the moment and let me clarify with a senior reviewer.

#Custom functions in public namespace

  1. OK
  2. OK

#Theme options

  1. OK

#Templates must follow WordPress Architecture.

  1. OK. I see that you already did the change. I am Sorry about extra work. The guidelines dont say explicitly that you cant use a base.php. But my understanding is that base.php architecture is not compatible with the guidelines requirements. Let me clarify with a senior reviewer.

#Notice on theme activation

  1. OK

#Displaying problems

  1. For long titles, on pages other than 'home' it still overlaps the content below, preventing the content from being selected. This is not a requirement, but it might be a good idea to apply a overflow: hidden for the box containing the title.
  2. OK
  3. OK

# Theme Options - Redux

  1. I dont know why it needs to be removed, but I know you do need to remove it. This request is from a senior reviewer, so I will ask him the reason.

Thank you,

Julien

#12 @britner
3 years ago

  1. I'll update and comb through files again. Although get_author_posts_url() is a core function and even in the docs they don't escape.. see here: http://codex.wordpress.org/Function_Reference/get_author_posts_url
  1. I removed the small line that told redux to fire the tracking. Even though it was commented out before and it wouldn't work because the tracking codes wasn't there. I still removed the commented line in the last update.

Thanks for your time!

Ben

#13 @Julien Klepatch
3 years ago

  1. Yes it's a core function but that does not mean it should not be escaped. I checked the function code and I haven't seen any esc_url() inside get_authors_posts_url:

https://core.trac.wordpress.org/browser/tags/4.0/src/wp-includes/author-template.php#L272

I have just seen that the variable returned is output from home_url(). Codex advises to use esc_url() after home_url():
http://codex.wordpress.org/Function_Reference/home_url

  1. Your theme definitely implements its own version of a custom header, so yes it needs to also implement the core custom header functionality. Also, I was not able to change the header background. Could you guide me ? I would like to check this option works. Finally, I am a bit confused between the logo option in 'Site Header' menu and the logo option in 'Logo options' menu. I didn't get to make it work in 'Site Header', but it worked in 'Logo options'.
  1. Great ! The reason it should be removed is that it is never going to be used. .

Thanks,

Julien

#14 @greenshady
3 years ago

There's no rule that a theme author must use the WordPress template hierarchy nor should they be. WordPress provides hooks for overwriting the hierarchy. The theme author used an appropriate filter hook (template_include) to change the hierarchy to use base.php. That's perfectly fine.

As for the custom header and custom footer, themes are generally required to use get_header() and get_footer() because of the get_header and get_footer hooks. As long as the theme author accounts for those hooks, it's OK.

#15 @britner
3 years ago

@Julien Klepatch
In the theme options > page title. You can adjust the page title background for the whole site.

In the theme options > home slider. You can also set the page title background but just for the home page.

In the theme options > site header. You can turn the "Enable Transparent header?" on which set a default for the site and means that the page title background will span up behind the logo.

in the theme options >site header you can set a logo for when the header is transparent, see here: http://themes.kadencethemes.com/pinnacle/ (notice the white logo)
You can also override the default and have a page that doesn't show a page title which would then show the logo that you set in the theme options > logo settings. See here: http://themes.kadencethemes.com/pinnacle/shop/ (notice the black logo)

Does that make some sense?

Ben

#16 @themetracbot
3 years ago

  • Summary changed from THEME: Pinnacle - 1.0.3 to THEME: Pinnacle - 1.0.4

Pinnacle - 1.0.4

Pinnacle is a bold theme with versatile options and multiple styles. This theme is loaded with features and tools that allow full creativity to be released into a unique site. Built with a modern flat design, its fully responsive layout make for easy navigation on mobile/tablet displays. Pinnacle is perfect for any kind of business, online store, portfolio, or personal site. It is fully compatible with woocommerce and gives you a unique layout for an ecommerce site. Pinnacle was built and designed by <a href="http://kadencethemes.com/">Kadence Themes</a>.

Theme URL - http://kadencethemes.com/product/pinnacle-free-theme/
Author URL - http://www.kadencethemes.com/

SVN - https://themes.svn.wordpress.org/pinnacle/1.0.4
ZIP - https://wordpress.org/themes/download/pinnacle.1.0.4.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=/pinnacle/1.0.3&new_path=/pinnacle/1.0.4

History:

Ticket Summary Status Resolution Owner
#20550 THEME: Pinnacle - 1.2.0 closed live grapplerulrich

(this ticket)

#23411 THEME: Pinnacle - 1.2.1 closed live emiluzelac
#23921 THEME: Pinnacle – 1.2.2 closed live jcastaneda
#24107 THEME: Pinnacle – 1.2.3 closed live jcastaneda
#24289 THEME: Pinnacle – 1.2.5 closed live jacenty3590
#24308 THEME: Pinnacle – 1.2.6 closed live jacenty3590
#24626 THEME: Pinnacle – 1.2.7 closed live karmatosed
#25383 THEME: Pinnaclele – 1.2.8 closed not-approved emiluzelac
#25442 THEME: Pinnacle – 1.2.8 closed live poena
#26484 THEME: Pinnacle – 1.3.0 closed live jcastaneda
#27201 THEME: Pinnacle – 1.3.1 closed live jcastaneda
#28491 THEME: Pinnacle – 1.3.2 closed live karmatosed
#29065 THEME: Pinnacle – 1.3.3 closed live Otto42
#29415 THEME: Pinnacle – 1.3.4 closed live emiluzelac
#29562 THEME: Pinnacle – 1.3.5 closed live Otto42
#30275 THEME: Pinnacle – 1.3.6 closed live jcastaneda
#31947 THEME: Pinnacle – 1.3.7 closed live themetracbot
#33856 THEME: Pinnacle – 1.3.8 closed live themetracbot
#33981 THEME: Pinnacle – 1.3.9 closed live themetracbot
#34678 THEME: Pinnacle – 1.4.0 closed live themetracbot
#34793 THEME: Pinnacle – 1.4.1 closed live themetracbot
#34839 THEME: Pinnacle – 1.4.2 closed live themetracbot
#35151 THEME: Pinnacle – 1.4.3 closed live themetracbot
#35258 THEME: Pinnacle – 1.4.4 closed live themetracbot
#36876 THEME: Pinnacle – 1.4.5 closed live themetracbot
#37644 THEME: Pinnacle – 1.4.6 closed live themetracbot
#41193 THEME: Pinnacle – 1.4.7 closed live themetracbot
#41321 THEME: Pinnacle – 1.4.8 closed live themetracbot
#41666 THEME: Pinnacle – 1.5.0 closed live themetracbot
#41684 THEME: Pinnacle – 1.5.1 closed live themetracbot
#41795 THEME: Pinnacle – 1.5.2 closed live themetracbot
#41889 THEME: Pinnacle – 1.5.3 closed live Otto42
#41978 THEME: Pinnacle – 1.5.4 closed live themetracbot
#42208 THEME: Pinnacle – 1.5.5 closed live themetracbot
#43384 THEME: Pinnacle – 1.5.6 closed live themetracbot
#44286 THEME: Pinnacle – 1.5.7 closed live themetracbot
#44483 THEME: Pinnacle – 1.5.8 closed live themetracbot
#49080 THEME: Pinnacle – 1.5.9 closed live themetracbot


https://themes.svn.wordpress.org/pinnacle/1.0.4/screenshot.png

#17 @Julien Klepatch
3 years ago

==>Ben,

Your theme looks fine for me

I'll ask admin for final review

Cheers,

Julien

greenshady=> thanks for your explanation

#18 @britner
3 years ago

Hey Thanks,
I was of the understanding that you approve the theme and a admin does the final?
Ben

#19 @britner
3 years ago

Hey Julien,

I realize you must be very busy but I don't understand this wait? If you want to push to a final review why not click the approve button so it can go to the final review?

Ben

#20 @greenshady
3 years ago

Yes, Julien, you need to mark the theme as approved. Otherwise, it'll never make it to the admins for final approval.

#21 @Julien Klepatch
3 years ago

  • Status changed from reviewing to approved

ok, thanks for your tip greenshady :)

Ben I marked your theme as approved

#22 @emiluzelac
3 years ago

  • Status changed from approved to reopened

Please move the following into functions file and enqueue from there.

<!--[if lt IE 9]>
      <script src="<?php echo get_template_directory_uri() . '/assets/js/vendor/respond.min.js';?>"></script>
<![endif]-->

Theme is using Redux framework. @Julien Klepatch please note that there's an ongoing issue with tracking (phoning home) with the framework and that must be solved before we can approve the theme.


Please don't remove add_filter('the_generator', '__return_false'); in https://themes.svn.wordpress.org/pinnacle/1.0.4/lib/cleanup.php


Also not sure the reason of this:

Clean up language_attributes() used in <html> tag or Add and remove body_class() classes


Mobile detect is plugin-territory, not reliable or really safe: https://themes.svn.wordpress.org/pinnacle/1.0.4/lib/mobile_detect.php


No https://themes.svn.wordpress.org/pinnacle/1.0.4/lib/shortcodes.php please.


@grapplerulrich please help @Julien Klepatch with this review, thanks!

#23 @emiluzelac
3 years ago

Additional:


Scripts must not be hardcoded, please move them into functions and enqueue from there.

<script type="text/javascript">jQuery( window ).load(function () {var $container = jQuery('#kad-blog-grid');$container.masonry({itemSelector: '.b_item'});});</script>

#24 @grapplerulrich
3 years ago

@emiluzelac - Sure, there should be no tracking code. Did you find some? Why would mobile detect be plugin territory?

@Julien Klepatch - Please assign yourself as a reviewer.

#25 @emiluzelac
3 years ago

  • Owner changed from Julien Klepatch to @…
  • Status changed from reopened to reviewing

#26 @emiluzelac
3 years ago

  • Owner changed from @… to Julien Klepatch

#27 @emiluzelac
3 years ago

Hi @grapplerulrich, I wasn't going into details with Redux, just noticed and gave heads-up.

Just like in core, mobile detect tools are not for frontend.

For example, there is this: http://codex.wordpress.org/Function_Reference/wp_is_mobile and it is not recommended https://core.trac.wordpress.org/ticket/26221, not sure how this specific mobile detect would be any different from one shipped with core?

#28 @britner
3 years ago

Hey @emiluzelac

  1. The included redux doesn't have any of the tracking code, It's all beed removed before uploading the theme, I'm well aware of how it works.
  1. The check for mobile allows to distinguish between phones and tablets. Which I was told the core doesn't. I realize it wouldn't work if used in junction with a caching plugin. (This is an optional feature for the front page and w3 total cache has a function to turn page caching off for the font page, some users like the feature.) But does that really make it not pass guidelines?
  1. Enqueue scripts for IE is there an accepted way? Twenty Fourteen Theme uses the same code I have in the header.php file. It's not enqueued in the function file.
  1. Just because the theme has a file named shortcodes doesn't mean it has any shortcodes. The theme check would have picked it up? Is there guidelines saying I can't have a file in my theme called shortcodes?

I do thank you for your time.

Ben

#29 @emiluzelac
3 years ago

  1. As noted, quick scan, the rest I will leave for the reviewer and mentor.
  2. Yes, it would definitely make is not pass, it does not belong in themes.
  3. Not really, Twenty Fourteen includes the HTML5 script:

<script src="<?php echo get_template_directory_uri(); ?>/js/html5.js"></script>

and that would be different from media queries for old browsers. That's the only type we allow via header.php

  1. You're enabling add_filter('widget_text', 'do_shortcode', 50); and sorry to say that would not be accepted either. Or anything else in same file.

See we gave HTML5.js a pass just so that old browsers would "understand" the HTML5 and to work, responsive and media queries is optional and older browser can work without them, but it will not be responsive of course. Not work properly? yes, but still work, while HTML5.js is a must, otherwise everything will be broken.

If there is anything else please ask away :)

#30 @greenshady
3 years ago

I'll chime in here on the mobile detection. We don't really have any guidelines that specifically say that such a thing is plugin territory. I actually believe mobile detection leans to being more theme territory. However, this is a pretty unique case.

There's 1175 lines of PHP code there dealing with headers and user agents. Frankly, I'm not going to go through that for security issues. We probably only have a handful of reviewers with the security know-how to review that code. Since the script is practically useless (unreliable and probably unsafe), I don't think anyone is going to jump on the opportunity to review it.

I'm going to agree with Emil and say remove it because I think it's more trouble than it's worth.

Last edited 3 years ago by greenshady (previous) (diff)

#31 @britner
3 years ago

  1. That is news to me. Can I do something like this instead? (literally just grabbed this from another bootstrap theme for an example)
function kadence_ie_support_header() {
    echo '<!--[if lt IE 9]>'. "\n";
    echo '<script src="' . esc_url( get_template_directory_uri() . '/inc/js/respond.min.js' ) . '"></script>'. "\n";
    echo '<![endif]-->'. "\n";
}
add_action( 'wp_head', 'kadence_ie_support_header', 10 );

Last: Mobile, so to me clear I can't use the core function then either?

Thanks again.

Ben

#32 @grapplerulrich
3 years ago

@emiluzelac - The guidelines say that a browser workaround script is allowed and I saw respond.js as a browser workaround script.

No hard coding of scripts, styles and Favicons unless a browser workaround script.

#33 @emiluzelac
3 years ago

Hey @britner, doing this from the functions will work and you are correct about not using mobile detects.

@grapplerulrich, it's just different wording that's all. Workaround would be as in html5.js or equal, respond falls into different category. See one makes site be operational and another one helps with media queries.

#34 @themetracbot
3 years ago

  • Summary changed from THEME: Pinnacle - 1.0.4 to THEME: Pinnacle - 1.0.5

Pinnacle - 1.0.5

Pinnacle is a bold theme with versatile options and multiple styles. This theme is loaded with features and tools that allow full creativity to be released into a unique site. Built with a modern flat design, its fully responsive layout make for easy navigation on mobile/tablet displays. Pinnacle is perfect for any kind of business, online store, portfolio, or personal site. It is fully compatible with woocommerce and gives you a unique layout for an ecommerce site. Pinnacle was built and designed by <a href="http://kadencethemes.com/">Kadence Themes</a>.

Theme URL - http://kadencethemes.com/product/pinnacle-free-theme/
Author URL - http://www.kadencethemes.com/

SVN - https://themes.svn.wordpress.org/pinnacle/1.0.5
ZIP - https://wordpress.org/themes/download/pinnacle.1.0.5.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=/pinnacle/1.0.4&new_path=/pinnacle/1.0.5

History:

Ticket Summary Status Resolution Owner
#20550 THEME: Pinnacle - 1.2.0 closed live grapplerulrich

(this ticket)

#23411 THEME: Pinnacle - 1.2.1 closed live emiluzelac
#23921 THEME: Pinnacle – 1.2.2 closed live jcastaneda
#24107 THEME: Pinnacle – 1.2.3 closed live jcastaneda
#24289 THEME: Pinnacle – 1.2.5 closed live jacenty3590
#24308 THEME: Pinnacle – 1.2.6 closed live jacenty3590
#24626 THEME: Pinnacle – 1.2.7 closed live karmatosed
#25383 THEME: Pinnaclele – 1.2.8 closed not-approved emiluzelac
#25442 THEME: Pinnacle – 1.2.8 closed live poena
#26484 THEME: Pinnacle – 1.3.0 closed live jcastaneda
#27201 THEME: Pinnacle – 1.3.1 closed live jcastaneda
#28491 THEME: Pinnacle – 1.3.2 closed live karmatosed
#29065 THEME: Pinnacle – 1.3.3 closed live Otto42
#29415 THEME: Pinnacle – 1.3.4 closed live emiluzelac
#29562 THEME: Pinnacle – 1.3.5 closed live Otto42
#30275 THEME: Pinnacle – 1.3.6 closed live jcastaneda
#31947 THEME: Pinnacle – 1.3.7 closed live themetracbot
#33856 THEME: Pinnacle – 1.3.8 closed live themetracbot
#33981 THEME: Pinnacle – 1.3.9 closed live themetracbot
#34678 THEME: Pinnacle – 1.4.0 closed live themetracbot
#34793 THEME: Pinnacle – 1.4.1 closed live themetracbot
#34839 THEME: Pinnacle – 1.4.2 closed live themetracbot
#35151 THEME: Pinnacle – 1.4.3 closed live themetracbot
#35258 THEME: Pinnacle – 1.4.4 closed live themetracbot
#36876 THEME: Pinnacle – 1.4.5 closed live themetracbot
#37644 THEME: Pinnacle – 1.4.6 closed live themetracbot
#41193 THEME: Pinnacle – 1.4.7 closed live themetracbot
#41321 THEME: Pinnacle – 1.4.8 closed live themetracbot
#41666 THEME: Pinnacle – 1.5.0 closed live themetracbot
#41684 THEME: Pinnacle – 1.5.1 closed live themetracbot
#41795 THEME: Pinnacle – 1.5.2 closed live themetracbot
#41889 THEME: Pinnacle – 1.5.3 closed live Otto42
#41978 THEME: Pinnacle – 1.5.4 closed live themetracbot
#42208 THEME: Pinnacle – 1.5.5 closed live themetracbot
#43384 THEME: Pinnacle – 1.5.6 closed live themetracbot
#44286 THEME: Pinnacle – 1.5.7 closed live themetracbot
#44483 THEME: Pinnacle – 1.5.8 closed live themetracbot
#49080 THEME: Pinnacle – 1.5.9 closed live themetracbot


https://themes.svn.wordpress.org/pinnacle/1.0.5/screenshot.png

#35 @britner
3 years ago

Ok, Updated with changes in requested areas.

  • Remove mobile check.
  • Move response.js to function.
  • Remove some things in cleanup.php
  • Remove shortcodes.php
  • Move some inline scripts to main js file.

Ben

#36 @britner
3 years ago

@grapplerulrich - Does this go back to Julien Klepatch? He has to approve again?

Ben

#37 @grapplerulrich
3 years ago

Yes, I would like to though check that he has not missed anything. I will do a review within the next 48 hours.

#38 @grapplerulrich
3 years ago

I started to look through the theme. It is enormous. It is going take me some time to look through it and understand what is happening with the different templates. Here are some of my initial findings.

Required

  • Variables need to be escaped on output template-portfolio-grid.php, template-feature.php, taxonomy-portfolio-type.php, front-page.php, field_kad_icons.php, field_kad_slides.php, woo-product-header.php, similarblog-carousel.php, portfolio-post-header.php, page-header.php, header-topbar.php, header.php and other places.
  • Please enqueue all scripts like in similarblog-carousel.php, recentblog-carousel.php and some other places.
  • Please make sure all of the functions are prefix consistently with pinnacle_
  • The contact form and map in the template-contact.php are plugin territory. You would best add support for specific plugins.
  • The blog templates are not allowed.
  • WordPress needs to be spelled correctly. I see it spelled incorrectly in the readme and the footer
  • Only presentational options are allowed in the metaboxes so no data/text/url are allowed to be entered which can be lost when switching themes.
  • Please correct the text domain in product-searchform.php, variable.php, entry-meta.php and other places.
  • Error in the admin area when changed the page template to Portfolio Grid
    Warning: array_slice() expects parameter 1 to be array, object given in /srv/www/theme-review/htdocs/wp-includes/class-wp-walker.php on line 288
    

Recommended

  • Please use the WordPress coding standards. It makes it a lot easier for me to read the code and give you feedback
  • I would also recommend trying to reduce the number of files.

Notes

  • What do the other page templates do and do they require any plugins?

#39 @britner
3 years ago

Required:

  1. no problem
  2. no problem
  3. no problem
  4. I have been given permission in the past (by chip) to have a contact template because being able to design the layout of a contact page was considered theme territory... I supposed thats changed?
  5. I am not aware of this? Why, is there an explanation? Many people love to have a blog template so they can set up custom blog pages...
  6. no problem
  7. data like what? It's already be cleared to allow visual things like videos, images? right? If it's theme specific layouts is it not part of the appearance?
  8. no problem
  9. ahh, yeah i'll fix.

Recomended:

  1. Anything specific your talking about?
  2. How is that good coding? If you need say the post meta data in multiple places why not make it it's own file? Files that are huge with lines and lines of code can make it more complicated?

Notes:
What other page templates are you talking about? You mean the feature? It doesn't need a plugin.

Ben

#40 @greenshady
3 years ago

I am not aware of this? Why, is there an explanation? Many people love to have a blog template so they can set up custom blog pages...

home.php is the blog template. If you're doing a single, custom blog template, it should be home.php. However, there is no reason that you can't have other templates to display the "blog" in different manners. There's some leeway with this, but the main blog template should not be a page template. It should be a home.php template.

I have been given permission in the past (by chip) to have a contact template because being able to design the layout of a contact page was considered theme territory... I supposed thats changed?

You can certainly create a contact page template. However, the functionality for the contact form itself is plugin territory and not allowed. Not to mention, the form code is insecure and needs an overhaul, even if we did allow it.

data like what? It's already be cleared to allow visual things like videos, images? right? If it's theme specific layouts is it not part of the appearance?

Presentation-related data. Videos, images, etc. are content. An option to use a 2-column vs. 1-column layout is presentational, for example.

#41 @grapplerulrich
3 years ago

Thanks Justin for your comments.

Anything specific your talking about?

A specific example would be the template-portfolio-grid.php. I tried to "clean" it up a bit. I did it till line 139 https://gist.github.com/grappler/3b1caec9303eb2d23bb6 I hope you can see that it is easier to read.

How is that good coding? If you need say the post meta data in multiple places why not make it it's own file? Files that are huge with lines and lines of code can make it more complicated?

I am not saying that you should move the code into single files. I was just wondering if every file was really need and if there was not a better way with less code. It is just a lot of code to review :)

Required

  • Image size handles should be prefixed e.g. init.php

@greenshady - How should a theme go about creating a design link this with plugins. The user needs to be able to define a specific contact form or address in the map.
https://slack-files.com/files-tmb/T024MFP4J-F0364HF0Q-ee330d/pasted_image_at_2014_12_12_04_32_pm_1024.png

#42 @greenshady
3 years ago

@greenshady - How should a theme go about creating a design link this with plugins. The user needs to be able to define a specific contact form or address in the map.

For contact forms, I recommend that theme authors actually create styles and test with all form elements. To me, that's akin to styling any other HTML element like <p>, <a>, and so on. A well-designed theme will pretty much work with any form, contact or otherwise. Now, a theme author can decide to choose to add some specific designs for any form plugin they wish.

As for a map, I don't care if that's a part of the theme. A customizer option or whatever to add a map is really fine by me if the theme author feels like it's an integral part of the theme. But, there's a line between plugins and themes, which is sometimes a bit blurry. But, you don't want to start jumping into developing a full plugin within a theme. My recommendation is to find one or a few map plugins and style for them.

If the form + map are tied together in some way, the theme author could always create a plugin to handle this.

#43 follow-up: @britner
3 years ago

Blog Template: OK, so blog template, your saying as long as I add a file called home.php (instead of just using the index.php file) I can also have my two blog templates?

Contact Form: I will remove the contact form (of course I have styling for all forms, it just makes having a contact form that is position on the page a certain way much easier if the user doesn't have to add a plugin. But then can I use a meta box for the user to add the plugin contact form plugin shortcode?

Metaboxes: This one I am very confused on... How does someone add a video post format without a native place to add the video like the "featured image" or in the same example a gallery or slider without a native place to add that gallery or slider? The images are part of the media library they are not lost when the user changes themes? Just the style/layout of the posts/pages which is what I though a theme is supposed to control. right? A link to a video doesn't mean the video is erased? If a theme can't add extra metaboxes to control the media library how do you get any extra layout styles? A layout becomes very limited if the only out put to a page is the "page content".

Coding Standards: I suppose since I wrote the code I wasn't writing the code to be styled. I didn't know that adding new spaces pre line was considered wordpress standards. I'll work on that.

Map: The map and contact form are not tied together but you do enter the map data, like there address and zoom levels in a meta box. So are you saying that is not ok either?

I certainly appreciate your time and helpful insights.

Ben

#44 in reply to: ↑ 43 @greenshady
3 years ago

Replying to britner:

Blog Template: OK, so blog template, your saying as long as I add a file called home.php (instead of just using the index.php file) I can also have my two blog templates?

You'd actually have 1 blog template (home.php) and 2 page templates in that case. I see no reason why you can't have templates like that in addition to the blog template. There are certainly many different methods to handling this. This would probably be fine.

Contact Form: I will remove the contact form (of course I have styling for all forms, it just makes having a contact form that is position on the page a certain way much easier if the user doesn't have to add a plugin. But then can I use a meta box for the user to add the plugin contact form plugin shortcode?

No, you can't use a meta box for that. Meta boxes in themes can only be presentational in nature. That would be content. When the user switched to another theme, their form output would disappear.

Metaboxes: This one I am very confused on... How does someone add a video post format without a native place to add the video like the "featured image" or in the same example a gallery or slider without a native place to add that gallery or slider?

The user adds them where they always add them, which is in the post editor.

The images are part of the media library they are not lost when the user changes themes?

No, images are not lost when users change themes. They can upload images regardless of theme.

Just the style/layout of the posts/pages which is what I though a theme is supposed to control. right?

That's right. It's just not what this conversation is about. We're not discussing style/layout at all. You can do whatever you want to with that.

A link to a video doesn't mean the video is erased?

No, but it means the link is lost and the video will no longer show in the post when the user switches to another theme.

If a theme can't add extra metaboxes to control the media library how do you get any extra layout styles? A layout becomes very limited if the only out put to a page is the "page content".

It's only limited by your coding skills and ingenuity. There are some things I personally wish WP would do as far as this goes, but I have no control over the direction of core WP. What we can do is simply apply the guidelines fairly to all themes. Our guidelines specifically prohibit themes handling the generation of user content. You might say that is our No. 1 guideline.

Map: The map and contact form are not tied together but you do enter the map data, like there address and zoom levels in a meta box. So are you saying that is not ok either?

I haven't looked at it in detail, but I doubt it's OK in a meta box. Why not simply add the map to the post content?

*

Basically, ask yourself this simple question: When a user changes themes, will the content they entered still appear?

1) If the answer is yes, you're good.
2) If the answer is no, you're not good.

#45 @britner
3 years ago

Hey @greenshady,
Thanks for the comments.

It's only limited by your coding skills and ingenuity. Is there a way with great coding skills and ingenuity that I can allow a user to add content above and below the page or post title? :)

I get that if someone has a video above the post title on my theme it won't translate to the next but I thought this was similar to the core "featured image" - which shows or doesn't show depending on the theme?

ok so I want to be able to provide post where a user can add a video or a group of images for a slider and have it show in a different place then the post content or excerpt. If I just add this option as part of a plugin and not make it required. does that work for the guidelines?

Thanks!

Ben

#46 @greenshady
3 years ago

It's only limited by your coding skills and ingenuity. Is there a way with great coding skills and ingenuity that I can allow a user to add content above and below the page or post title? :)

That's a much longer conversation. I'm not sure I could explain that without writing several tutorials and a lot of code testing. :)

You are welcome to check out some of my code that's similar to that. It may not do exactly what you want, but it's all GPL, so feel free to fork and build what you want.

I get that if someone has a video above the post title on my theme it won't translate to the next but I thought this was similar to the core "featured image" - which shows or doesn't show depending on the theme?

It is similar but not exactly the same. That video will work with your theme and only your theme. Featured images will work with many themes. Core WP has set a standard for storing that data and has an appropriate API for getting the data regardless of theme. If your theme stored the feature image in a different way from core, we wouldn't allow that either.

What you really need to do is go to core Trac and open a new ticket for featured videos if that's something you'd like to see. I'm sure a lot of theme authors would find it useful.

ok so I want to be able to provide post where a user can add a video or a group of images for a slider and have it show in a different place then the post content or excerpt. If I just add this option as part of a plugin and not make it required. does that work for the guidelines?

Definitely. A plugin would be a great place for this because the content would no longer be tied to your theme. There are even some existing plugins in the repository for such things. I don't know their links offhand, but you can search around or build your own.

#47 @grapplerulrich
3 years ago

Hi Ben,

Will you be uploading an update with the changes for the noted requirements?

We have a 7 day no-response closing policy. Please just let us know that you are working on a update.

Thanks :)

#48 @britner
3 years ago

Hey,
Yes working on the update. I should have it uploaded tomorrow. Thanks!

Ben

#49 @themetracbot
3 years ago

  • Summary changed from THEME: Pinnacle - 1.0.5 to THEME: Pinnacle - 1.1.6

Pinnacle - 1.1.6

Pinnacle is a bold theme with versatile options and multiple styles. This theme is loaded with features and tools that allow full creativity to be released into a unique site. Built with a modern flat design, its fully responsive layout make for easy navigation on mobile/tablet displays. Pinnacle is perfect for any kind of business, online store, portfolio, or personal site. It is fully compatible with woocommerce and gives you a unique layout for an ecommerce site. Pinnacle was built and designed by <a href="http://kadencethemes.com/">Kadence Themes</a>.

Theme URL - http://kadencethemes.com/product/pinnacle-free-theme/
Author URL - http://www.kadencethemes.com/

SVN - https://themes.svn.wordpress.org/pinnacle/1.1.6
ZIP - https://wordpress.org/themes/download/pinnacle.1.1.6.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=/pinnacle/1.0.5&new_path=/pinnacle/1.1.6

History:

Ticket Summary Status Resolution Owner
#20550 THEME: Pinnacle - 1.2.0 closed live grapplerulrich

(this ticket)

#23411 THEME: Pinnacle - 1.2.1 closed live emiluzelac
#23921 THEME: Pinnacle – 1.2.2 closed live jcastaneda
#24107 THEME: Pinnacle – 1.2.3 closed live jcastaneda
#24289 THEME: Pinnacle – 1.2.5 closed live jacenty3590
#24308 THEME: Pinnacle – 1.2.6 closed live jacenty3590
#24626 THEME: Pinnacle – 1.2.7 closed live karmatosed
#25383 THEME: Pinnaclele – 1.2.8 closed not-approved emiluzelac
#25442 THEME: Pinnacle – 1.2.8 closed live poena
#26484 THEME: Pinnacle – 1.3.0 closed live jcastaneda
#27201 THEME: Pinnacle – 1.3.1 closed live jcastaneda
#28491 THEME: Pinnacle – 1.3.2 closed live karmatosed
#29065 THEME: Pinnacle – 1.3.3 closed live Otto42
#29415 THEME: Pinnacle – 1.3.4 closed live emiluzelac
#29562 THEME: Pinnacle – 1.3.5 closed live Otto42
#30275 THEME: Pinnacle – 1.3.6 closed live jcastaneda
#31947 THEME: Pinnacle – 1.3.7 closed live themetracbot
#33856 THEME: Pinnacle – 1.3.8 closed live themetracbot
#33981 THEME: Pinnacle – 1.3.9 closed live themetracbot
#34678 THEME: Pinnacle – 1.4.0 closed live themetracbot
#34793 THEME: Pinnacle – 1.4.1 closed live themetracbot
#34839 THEME: Pinnacle – 1.4.2 closed live themetracbot
#35151 THEME: Pinnacle – 1.4.3 closed live themetracbot
#35258 THEME: Pinnacle – 1.4.4 closed live themetracbot
#36876 THEME: Pinnacle – 1.4.5 closed live themetracbot
#37644 THEME: Pinnacle – 1.4.6 closed live themetracbot
#41193 THEME: Pinnacle – 1.4.7 closed live themetracbot
#41321 THEME: Pinnacle – 1.4.8 closed live themetracbot
#41666 THEME: Pinnacle – 1.5.0 closed live themetracbot
#41684 THEME: Pinnacle – 1.5.1 closed live themetracbot
#41795 THEME: Pinnacle – 1.5.2 closed live themetracbot
#41889 THEME: Pinnacle – 1.5.3 closed live Otto42
#41978 THEME: Pinnacle – 1.5.4 closed live themetracbot
#42208 THEME: Pinnacle – 1.5.5 closed live themetracbot
#43384 THEME: Pinnacle – 1.5.6 closed live themetracbot
#44286 THEME: Pinnacle – 1.5.7 closed live themetracbot
#44483 THEME: Pinnacle – 1.5.8 closed live themetracbot
#49080 THEME: Pinnacle – 1.5.9 closed live themetracbot


https://themes.svn.wordpress.org/pinnacle/1.1.6/screenshot.png

#50 @britner
3 years ago

@grapplerulrich to be clear for this thread and because Julian hasn't written anything for a while. The next step in the review is for Julian to confirm that I've changed the issues that were brought up by you and greenshady?

Thanks, hope your holidays are going well.

Ben

#51 @emiluzelac
3 years ago

Yes, or Ulrich, it doesn't really matter :)

#52 @grapplerulrich
3 years ago

Review

It looks good. I could not find anything other then these few small things.

@Justin/@Emil - Please could you have a look at it because it is a large theme and I may have missed something.

Required

  • pinnacle_main_class() pinnacle_category_layout_css() needs to be escaped on output
  • Text not internationalized in product-image.php line 51
  • This code in wpml-config.xml does make sense as the theme has no custom post types and taxonomies.
        <custom-types>
            <custom-type translate="1">portfolio</custom-type>
            <custom-type translate="1">staff</custom-type>
            <custom-type translate="1">testimonial</custom-type>
        </custom-types>
        <taxonomies>
            <taxonomy translate="1">portfolio-type</taxonomy>
            <taxonomy translate="1">testimonial-group</taxonomy>
            <taxonomy translate="1">staff-group</taxonomy>
        </taxonomies>      
    


#53 @themetracbot
3 years ago

  • Summary changed from THEME: Pinnacle - 1.1.6 to THEME: Pinnacle - 1.1.7

Pinnacle - 1.1.7

Pinnacle is a bold theme with versatile options and multiple styles. This theme is loaded with features and tools that allow full creativity to be released into a unique site. Built with a modern flat design, its fully responsive layout make for easy navigation on mobile/tablet displays. Pinnacle is perfect for any kind of business, online store, portfolio, or personal site. It is fully compatible with woocommerce and gives you a unique layout for an ecommerce site. Pinnacle was built and designed by <a href="http://kadencethemes.com/">Kadence Themes</a>.

Theme URL - http://kadencethemes.com/product/pinnacle-free-theme/
Author URL - http://www.kadencethemes.com/

SVN - https://themes.svn.wordpress.org/pinnacle/1.1.7
ZIP - https://wordpress.org/themes/download/pinnacle.1.1.7.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=/pinnacle/1.1.6&new_path=/pinnacle/1.1.7

History:

Ticket Summary Status Resolution Owner
#20550 THEME: Pinnacle - 1.2.0 closed live grapplerulrich

(this ticket)

#23411 THEME: Pinnacle - 1.2.1 closed live emiluzelac
#23921 THEME: Pinnacle – 1.2.2 closed live jcastaneda
#24107 THEME: Pinnacle – 1.2.3 closed live jcastaneda
#24289 THEME: Pinnacle – 1.2.5 closed live jacenty3590
#24308 THEME: Pinnacle – 1.2.6 closed live jacenty3590
#24626 THEME: Pinnacle – 1.2.7 closed live karmatosed
#25383 THEME: Pinnaclele – 1.2.8 closed not-approved emiluzelac
#25442 THEME: Pinnacle – 1.2.8 closed live poena
#26484 THEME: Pinnacle – 1.3.0 closed live jcastaneda
#27201 THEME: Pinnacle – 1.3.1 closed live jcastaneda
#28491 THEME: Pinnacle – 1.3.2 closed live karmatosed
#29065 THEME: Pinnacle – 1.3.3 closed live Otto42
#29415 THEME: Pinnacle – 1.3.4 closed live emiluzelac
#29562 THEME: Pinnacle – 1.3.5 closed live Otto42
#30275 THEME: Pinnacle – 1.3.6 closed live jcastaneda
#31947 THEME: Pinnacle – 1.3.7 closed live themetracbot
#33856 THEME: Pinnacle – 1.3.8 closed live themetracbot
#33981 THEME: Pinnacle – 1.3.9 closed live themetracbot
#34678 THEME: Pinnacle – 1.4.0 closed live themetracbot
#34793 THEME: Pinnacle – 1.4.1 closed live themetracbot
#34839 THEME: Pinnacle – 1.4.2 closed live themetracbot
#35151 THEME: Pinnacle – 1.4.3 closed live themetracbot
#35258 THEME: Pinnacle – 1.4.4 closed live themetracbot
#36876 THEME: Pinnacle – 1.4.5 closed live themetracbot
#37644 THEME: Pinnacle – 1.4.6 closed live themetracbot
#41193 THEME: Pinnacle – 1.4.7 closed live themetracbot
#41321 THEME: Pinnacle – 1.4.8 closed live themetracbot
#41666 THEME: Pinnacle – 1.5.0 closed live themetracbot
#41684 THEME: Pinnacle – 1.5.1 closed live themetracbot
#41795 THEME: Pinnacle – 1.5.2 closed live themetracbot
#41889 THEME: Pinnacle – 1.5.3 closed live Otto42
#41978 THEME: Pinnacle – 1.5.4 closed live themetracbot
#42208 THEME: Pinnacle – 1.5.5 closed live themetracbot
#43384 THEME: Pinnacle – 1.5.6 closed live themetracbot
#44286 THEME: Pinnacle – 1.5.7 closed live themetracbot
#44483 THEME: Pinnacle – 1.5.8 closed live themetracbot
#49080 THEME: Pinnacle – 1.5.9 closed live themetracbot


https://themes.svn.wordpress.org/pinnacle/1.1.7/screenshot.png

#54 @britner
3 years ago

Ok,
I've updated the theme with the requested changes.

Ben

#55 @emiluzelac
3 years ago

  • Cc greenshady added

#56 @britner
3 years ago

  1. Is it against guidelines to define a relative plugin path?
  2. ok, easy enough, where is this in the guidelines though?
  3. No it's not I've removed the core tracking function and remove the call for it. You can test the theme there isn't any tracking.
  4. Is this a guideline too? Where?

Ben

#57 @emiluzelac
3 years ago

Case-by-case, that is in guidelines :)

Last edited 3 years ago by emiluzelac (previous) (diff)

#58 @themetracbot
3 years ago

  • Summary changed from THEME: Pinnacle - 1.1.7 to THEME: Pinnacle - 1.1.8

Pinnacle - 1.1.8

Pinnacle is a bold theme with versatile options and multiple styles. This theme is loaded with features and tools that allow full creativity to be released into a unique site. Built with a modern flat design, its fully responsive layout make for easy navigation on mobile/tablet displays. Pinnacle is perfect for any kind of business, online store, portfolio, or personal site. It is fully compatible with woocommerce and gives you a unique layout for an ecommerce site. Pinnacle was built and designed by <a href="http://kadencethemes.com/">Kadence Themes</a>.

Theme URL - http://kadencethemes.com/product/pinnacle-free-theme/
Author URL - http://www.kadencethemes.com/

SVN - https://themes.svn.wordpress.org/pinnacle/1.1.8
ZIP - https://wordpress.org/themes/download/pinnacle.1.1.8.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=/pinnacle/1.1.7&new_path=/pinnacle/1.1.8

History:

Ticket Summary Status Resolution Owner
#20550 THEME: Pinnacle - 1.2.0 closed live grapplerulrich

(this ticket)

#23411 THEME: Pinnacle - 1.2.1 closed live emiluzelac
#23921 THEME: Pinnacle – 1.2.2 closed live jcastaneda
#24107 THEME: Pinnacle – 1.2.3 closed live jcastaneda
#24289 THEME: Pinnacle – 1.2.5 closed live jacenty3590
#24308 THEME: Pinnacle – 1.2.6 closed live jacenty3590
#24626 THEME: Pinnacle – 1.2.7 closed live karmatosed
#25383 THEME: Pinnaclele – 1.2.8 closed not-approved emiluzelac
#25442 THEME: Pinnacle – 1.2.8 closed live poena
#26484 THEME: Pinnacle – 1.3.0 closed live jcastaneda
#27201 THEME: Pinnacle – 1.3.1 closed live jcastaneda
#28491 THEME: Pinnacle – 1.3.2 closed live karmatosed
#29065 THEME: Pinnacle – 1.3.3 closed live Otto42
#29415 THEME: Pinnacle – 1.3.4 closed live emiluzelac
#29562 THEME: Pinnacle – 1.3.5 closed live Otto42
#30275 THEME: Pinnacle – 1.3.6 closed live jcastaneda
#31947 THEME: Pinnacle – 1.3.7 closed live themetracbot
#33856 THEME: Pinnacle – 1.3.8 closed live themetracbot
#33981 THEME: Pinnacle – 1.3.9 closed live themetracbot
#34678 THEME: Pinnacle – 1.4.0 closed live themetracbot
#34793 THEME: Pinnacle – 1.4.1 closed live themetracbot
#34839 THEME: Pinnacle – 1.4.2 closed live themetracbot
#35151 THEME: Pinnacle – 1.4.3 closed live themetracbot
#35258 THEME: Pinnacle – 1.4.4 closed live themetracbot
#36876 THEME: Pinnacle – 1.4.5 closed live themetracbot
#37644 THEME: Pinnacle – 1.4.6 closed live themetracbot
#41193 THEME: Pinnacle – 1.4.7 closed live themetracbot
#41321 THEME: Pinnacle – 1.4.8 closed live themetracbot
#41666 THEME: Pinnacle – 1.5.0 closed live themetracbot
#41684 THEME: Pinnacle – 1.5.1 closed live themetracbot
#41795 THEME: Pinnacle – 1.5.2 closed live themetracbot
#41889 THEME: Pinnacle – 1.5.3 closed live Otto42
#41978 THEME: Pinnacle – 1.5.4 closed live themetracbot
#42208 THEME: Pinnacle – 1.5.5 closed live themetracbot
#43384 THEME: Pinnacle – 1.5.6 closed live themetracbot
#44286 THEME: Pinnacle – 1.5.7 closed live themetracbot
#44483 THEME: Pinnacle – 1.5.8 closed live themetracbot
#49080 THEME: Pinnacle – 1.5.9 closed live themetracbot


https://themes.svn.wordpress.org/pinnacle/1.1.8/screenshot.png

#59 @britner
3 years ago

Ok,
I've updated the theme with the requested changes.
Ben

#60 @britner
3 years ago

Hey,
@grapplerulrich or anyone?
It seems @Julien Klepatch isn't going to mark this around to finish this, he hasn't written anything in 3 months. Any chance you can finish this?

Thanks,
Ben

#61 @grapplerulrich
3 years ago

  • Cc emiluzelac added

@emiluzelac can you please re assign this theme to me?

I will have one more look over and then approve the theme.

#62 @emiluzelac
3 years ago

  • Owner changed from Julien Klepatch to grapplerulrich

#63 @britner
3 years ago

@grapplerulrich, I really don't mean to be rude or pushy, just seems I can only get something to happen on this if I post here and ask.
It's been a month since I put forth the update, three months since you've been a mentor for this review. I've changed every file in the theme so it's easier to read. Is there anything I can do to help you in reviewing this theme?

Thanks,

Ben

#64 @grapplerulrich
3 years ago

@britner - I am extremely sorry for the delay. I was bogged down and did not find a large enough time block.

I was planning of approving the theme but I found a few things doing the last minute checks.

Required

  • WordPress is not spelled correctly in the settings.
  • The "Custom CSS" option does not fulfill the level of sanitization that is required. It would be best to remove it.

Recommended

  • I was confused for what the Portfolio, Shop and Product settings were for. Perhaps you could mention what plugins you support and even hide the options if the plugin is not installed.

#65 @britner
3 years ago

Custom css, in the theme options? That whole box is sanitized and validated by the redux framework. What part of it doesn't meet requirements?

Ben

#66 @britner
3 years ago

Really frustrated here...

Where are these requirements, where is there a guideline doc showing how the reviewers want custom css sanitized? I'll be happy to check, I'm happy to add something if there is something missing? But I really don't understand that you just say they don't fulfill some requirements and the solution is to remove?

#68 @britner
3 years ago

ok, so what is missing in the theme?
It is all sanitized. It goes through a sanitize before it's saved.

Ben

#69 @emiluzelac
3 years ago

Not sure bud, I just linked the requirements, Ulrich will help.

#70 @greenshady
3 years ago

Sanitizing is one thing. Sanitizing correctly is another. If I'm following the code correctly (I'm unfamiliar with Redux, so correct me if I'm wrong), this is the code that's ultimately used to sanitize the CSS:

$data = wp_filter_nohtml_kses( $data );
$data = str_replace( '&gt;', '>', $data );

Honestly, that doesn't really cut it. Core WP does not have any built-in functions for sanitizing and validating CSS. It doesn't look like Redux does either. For something like that, you'd need CSSTidy or another library. See: https://github.com/Cerdic/CSSTidy

Also, unless I'm mistaken, you're using the manage_options capability to show the theme options. This is the incorrect capability for any theme option. You should be using edit_theme_options. For permission to add custom CSS in particular, you'd probably want to go with edit_themes.

The best course of action is to simply remove the option and allow plugins to handle this feature, especially since this doesn't seem to be an option that's really needed within your theme. However, you must meet the TRT recommendations for proper sanitization if you do decide to keep this option.

Last edited 3 years ago by greenshady (previous) (diff)

#71 @britner
3 years ago

Ok sweet that is something to work with, any examples of themes using csstidy? Or a plugin?

I feel that a css box for themes is one of the most important theme options. I understand that you disagree but I would like to make this work.

And I can easily change the permissions level.

Also out of curiosity what is the security risk? Is this an issue of people putting something into there custom css boxes?

Ben

#72 @britner
3 years ago

Also redux has an ace editor option. Would that be allowed? That has a more advanced syntax checker.

See demo: http://reduxframework.com/

Ben

#73 @britner
3 years ago

I just looked through this plugin: https://wordpress.org/plugins/simple-custom-css/

I must be missing what it's doing different?

Ben

#75 @britner
3 years ago

Does that mean your saying simple custom css isn't doing it right? Or does that mean the only way to get it approved is to use automattic's method with csstidy?

Also if you can't or don't want to answer about what the securty risk is? And what other options are. who is a good person to ask that knows about the custom css guidelines?

#76 @britner
3 years ago

ok, so I can get csstidy working fine, I really wish I knew why this parsing is necessary and who is heading up this guideline so I can learn about other options and who else is doing this besides automattic.

I'm am really interested in getting my theme live though so if someone can confirm this qualifies for the "level of sanitization that is required" that would be awesome.

Also note that if you recommend this to someone else they will have to tear out a bunch of functions that don't pass theme check in the csstidy code.

Ben

#77 @themetracbot
3 years ago

  • Summary changed from THEME: Pinnacle - 1.1.8 to THEME: Pinnacle - 1.1.9

Pinnacle - 1.1.9

Pinnacle is a bold theme with versatile options and multiple styles. This theme is loaded with features and tools that allow full creativity to be released into a unique site. Built with a modern flat design, its fully responsive layout make for easy navigation on mobile/tablet displays. Pinnacle is perfect for any kind of business, online store, portfolio, or personal site. It is fully compatible with woocommerce and gives you a unique layout for an ecommerce site. Pinnacle was built and designed by <a href="http://kadencethemes.com/">Kadence Themes</a>.

Theme URL - http://kadencethemes.com/product/pinnacle-free-theme/
Author URL - http://www.kadencethemes.com/

SVN - https://themes.svn.wordpress.org/pinnacle/1.1.9
ZIP - https://wordpress.org/themes/download/pinnacle.1.1.9.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=/pinnacle/1.1.8&new_path=/pinnacle/1.1.9

History:

Ticket Summary Status Resolution Owner
#20550 THEME: Pinnacle - 1.2.0 closed live grapplerulrich

(this ticket)

#23411 THEME: Pinnacle - 1.2.1 closed live emiluzelac
#23921 THEME: Pinnacle – 1.2.2 closed live jcastaneda
#24107 THEME: Pinnacle – 1.2.3 closed live jcastaneda
#24289 THEME: Pinnacle – 1.2.5 closed live jacenty3590
#24308 THEME: Pinnacle – 1.2.6 closed live jacenty3590
#24626 THEME: Pinnacle – 1.2.7 closed live karmatosed
#25383 THEME: Pinnaclele – 1.2.8 closed not-approved emiluzelac
#25442 THEME: Pinnacle – 1.2.8 closed live poena
#26484 THEME: Pinnacle – 1.3.0 closed live jcastaneda
#27201 THEME: Pinnacle – 1.3.1 closed live jcastaneda
#28491 THEME: Pinnacle – 1.3.2 closed live karmatosed
#29065 THEME: Pinnacle – 1.3.3 closed live Otto42
#29415 THEME: Pinnacle – 1.3.4 closed live emiluzelac
#29562 THEME: Pinnacle – 1.3.5 closed live Otto42
#30275 THEME: Pinnacle – 1.3.6 closed live jcastaneda
#31947 THEME: Pinnacle – 1.3.7 closed live themetracbot
#33856 THEME: Pinnacle – 1.3.8 closed live themetracbot
#33981 THEME: Pinnacle – 1.3.9 closed live themetracbot
#34678 THEME: Pinnacle – 1.4.0 closed live themetracbot
#34793 THEME: Pinnacle – 1.4.1 closed live themetracbot
#34839 THEME: Pinnacle – 1.4.2 closed live themetracbot
#35151 THEME: Pinnacle – 1.4.3 closed live themetracbot
#35258 THEME: Pinnacle – 1.4.4 closed live themetracbot
#36876 THEME: Pinnacle – 1.4.5 closed live themetracbot
#37644 THEME: Pinnacle – 1.4.6 closed live themetracbot
#41193 THEME: Pinnacle – 1.4.7 closed live themetracbot
#41321 THEME: Pinnacle – 1.4.8 closed live themetracbot
#41666 THEME: Pinnacle – 1.5.0 closed live themetracbot
#41684 THEME: Pinnacle – 1.5.1 closed live themetracbot
#41795 THEME: Pinnacle – 1.5.2 closed live themetracbot
#41889 THEME: Pinnacle – 1.5.3 closed live Otto42
#41978 THEME: Pinnacle – 1.5.4 closed live themetracbot
#42208 THEME: Pinnacle – 1.5.5 closed live themetracbot
#43384 THEME: Pinnacle – 1.5.6 closed live themetracbot
#44286 THEME: Pinnacle – 1.5.7 closed live themetracbot
#44483 THEME: Pinnacle – 1.5.8 closed live themetracbot
#49080 THEME: Pinnacle – 1.5.9 closed live themetracbot


https://themes.svn.wordpress.org/pinnacle/1.1.9/screenshot.png

#78 @greenshady
3 years ago

We might have to hold off on this based on the discussion in the comments here: https://make.wordpress.org/themes/2015/02/03/theme-review-chat-notes

In particular, this comment: https://make.wordpress.org/themes/2015/02/03/theme-review-chat-notes/#comment-40882

There's only a handful of people who have the authority to overrule a theme review team decision. That is one of them. Keeping this option is going to be an uphill battle. I'd highly recommend getting involved with that discussion if you feel like this is something you must keep. I believe it's fair to hear all sides. My primary concern is security. If I have one of the lead developers of WP telling me it's not safe, I tend to listen.

#79 @britner
3 years ago

More delays. I'm overjoyed...

I really don't see the logic in any of the discussion. Almost all the plugins for custom css are using the same code that themes have been. So if this is really a security issue why push it off the theme review and on plugin review. It's a million times easer to get something passed on a plugin review. If this is truly about doing it the right way they why in the world would you not want theme developers raising the standards. Just come up with a approved way and let the theme developers decide if they want to add.

If csstidy is the best way why when I add it does it still not pass theme review? I can't find another plugin aside from jetpack that has better security then this theme does now??? Why is the discussion about this stopping my review! I'm adding the security you want, this makes no sense.

Ben

#80 @greenshady
3 years ago

We have absolutely zero control over plugins on WordPess.org. None whatsoever. This is the Theme Review Team. What plugins are allowed to do is completely out of our control.

TRT also tries to hold theme authors to a higher standard than any other in the WordPress world. We constantly argue about the details of this, sometimes on a daily basis. One thing we all agree on is that we want the best themes we can get in our repository. That often means saying NO to a lot of things. Even as admin, I've been overruled by the team on features that I wanted to have in my own themes. So, this isn't all about you. Our goal is to do what's best for the users.

Security is only half of the issue. The other half is whether such a feature is considered theme or plugin territory.

There are several reviews being held up because of this. This is the best course of action because it's a lot harder to remove an option than it is to add one later.

If you want your theme to go on through the process quickly, simply remove the feature. Then, re-add it in the next version once the dust has settled on this discussion and we decide that it's allowed. This is not a make-or-break feature for your theme. Truth be told, it has zero bearing on the usefulness of the theme. So, not having the option in your first release isn't going to be a problem for you or your users.

I also encourage getting involved in the discussion on the post I linked to.

#81 @britner
3 years ago

I'm only pointing to plugin reviews to make a point that pushing this into the "hands" of plugins doesn't get our end goal of secure sites. And considering how many themes have custom css boxes right now it only makes sense to promote doing it in a secure way. Not cutting it off because it inconveniences the review team or there are people who think it makes sense for plugin to have this. Just talk to users. With anything there is going to be a divide I don't think that means you withhold the option for themes.

I really don't thing it's right for a debate over what is plugin and theme territory to hold up current themes that were submitted under a set of guidelines. If it's not official it shouldn't be holding up reviews. Do the discussion, come up with guidelines then enforce them. Why send me on a crazy research project to figure out how to implement the css the way you want and then not approve... How is that far?

As for plugin territory... Considering themes control the appearance of a site I don't see how this would fall into plugin territory. Theme css boxes are for people who want to one small change, it's immensely helpful for support and it's a feature that many many people who don't need to write a ton of css take advantage of.

As for this being the first official release. If it where that easy I would remove the feature, hundreds already use this theme and already have css . This is a public forum and I've had the theme on my site for some time. Considering my other theme has custom css box wouldn't have guesses this would be an issue. Especially since I submitted this in the beginning of September.

So this brings us to a really important point, I've already started writing the update for my other theme where it will adopted csstidy for the custom css box. It would be insane if I had to remove it. What do I tell everyone using the theme that has css in there custom css box? You need to solve this in a way that makes sense.

Ben

#82 @greenshady
3 years ago

Let me be perfectly clear so that we're on the same page. What's going on in this ticket is not a debate. It's not a discussion about a pending guideline. I'm simply conveying the situation to you. It so happens that your theme is under review while this discussion is happening. Unfortunately, the forces of the universe are beyond my control. Fate has dealt you a crap hand this round. Try to make the best of it.

You have been given the link to the post where this discussion is currently taking place. Typically, we'd have discussions in-ticket. However, this is a unique case because it has an effect on a number of themes. That's why we've opened it up on the Make Themes blog.

I've already spent a considerable amount of time trying to help you today by responding in this ticket and getting in touch with the Redux author about the situation. This is time I spent trying to help instead of doing paid work or enjoying time with my family. So, anytime someone starts talking about things being "fair", I start thinking how my life is unfair and lose track of the important bits of the conversation.

I have given you the option to move forward and get this theme approved quickly. You have chosen not to take that option. However, it is still available to you if you choose to pursue it.

#83 @britner
3 years ago

"Far" was a poor choice in words, I certainly don't want to pull you away from your family. I am just mad, trying not to be but I don't think it's right that adding the requested security isn't moving this review along. From the discussions about this I only see that you are supposed to be discouraging the use of css and forcing sanitization. Where then is the discussion about this being plugin territory? as you said this is two things?

I have posted on the discussion. If there is something else I need to do let me know.

If you have any thoughts on how I could remove this and everyone using the theme wouldn't lose there css I would appreciate it.

Also since I've never been through a new guideline being discussed where would I go to see the progression? Like how does it get decided?

Ben

#84 @grapplerulrich
3 years ago

I'm only pointing to plugin reviews to make a point that pushing this into the "hands" of plugins doesn't get our end goal of secure sites.

That was one of my arguments when we were discussing this.

Also since I've never been through a new guideline being discussed where would I go to see the progression? Like how does it get decided?

The best place is to follow the make.wordpress.org/themes blog. We have a weekly meeting on Tuesdays and a there is a post beforehand announcing the topics we are going to discuss and then after the meeting the meeting notes are also posted.

If you have any thoughts on how I could remove this and everyone using the theme wouldn't lose there css I would appreciate it.

An idea that Justin had was to make the Custom CSS box read only so that people could copy their code out to other plugins.

Plugins that I think we can recommend which are not Jetpack are
https://wordpress.org/plugins/jp-custom-css/ and https://wordpress.org/plugins/reaktiv-css-builder/

I am not sure if you have seen but it looks like that you only need to update ReduxFramework https://github.com/ReduxFramework/redux-framework/commit/d6550686eb99a740b9a346cf33083fddab7c8c7b

#85 @britner
3 years ago

An idea that Justin had was to make the Custom CSS box read only so that people could copy their code out to other plugins.

I see some downsides, like you would then need to have a clear css option if/when the user wanted to move to a plugin and edit something. The support nightmare of getting everyone to download a plugin then copy the css over doesn't sound fun to me. Overall I really think css boxes should be in themes, I will wait.

Thanks for the link, I did see the dory updated redux it's not really anything different from whats in the theme now but I can update it if you think I need to.

Ben

#86 @britner
3 years ago

@grapplerulrich,
Based on the post that Justin wrote today is there anything I need to change for this theme to pass review?
Thanks,

Ben

#87 @greenshady
3 years ago

Yep, sorry about all the holdups on this one. We've finally made the decision about what we expect from custom CSS boxes. This review can move forward now.

#88 @themetracbot
3 years ago

  • Summary changed from THEME: Pinnacle - 1.1.9 to THEME: Pinnacle - 1.2.0

Pinnacle - 1.2.0

Pinnacle is a bold theme with versatile options and multiple styles. This theme is loaded with features and tools that allow full creativity to be released into a unique site. Built with a modern flat design, its fully responsive layout make for easy navigation on mobile/tablet displays. Pinnacle is perfect for any kind of business, online store, portfolio, or personal site. It is fully compatible with woocommerce and gives you a unique layout for an ecommerce site. Pinnacle was built and designed by <a href="http://kadencethemes.com/">Kadence Themes</a>.

Theme URL - http://kadencethemes.com/product/pinnacle-free-theme/
Author URL - http://www.kadencethemes.com/

SVN - https://themes.svn.wordpress.org/pinnacle/1.2.0
ZIP - https://wordpress.org/themes/download/pinnacle.1.2.0.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=/pinnacle/1.1.9&new_path=/pinnacle/1.2.0

History:

Ticket Summary Status Resolution Owner
#20550 THEME: Pinnacle - 1.2.0 closed live grapplerulrich

(this ticket)

#23411 THEME: Pinnacle - 1.2.1 closed live emiluzelac
#23921 THEME: Pinnacle – 1.2.2 closed live jcastaneda
#24107 THEME: Pinnacle – 1.2.3 closed live jcastaneda
#24289 THEME: Pinnacle – 1.2.5 closed live jacenty3590
#24308 THEME: Pinnacle – 1.2.6 closed live jacenty3590
#24626 THEME: Pinnacle – 1.2.7 closed live karmatosed
#25383 THEME: Pinnaclele – 1.2.8 closed not-approved emiluzelac
#25442 THEME: Pinnacle – 1.2.8 closed live poena
#26484 THEME: Pinnacle – 1.3.0 closed live jcastaneda
#27201 THEME: Pinnacle – 1.3.1 closed live jcastaneda
#28491 THEME: Pinnacle – 1.3.2 closed live karmatosed
#29065 THEME: Pinnacle – 1.3.3 closed live Otto42
#29415 THEME: Pinnacle – 1.3.4 closed live emiluzelac
#29562 THEME: Pinnacle – 1.3.5 closed live Otto42
#30275 THEME: Pinnacle – 1.3.6 closed live jcastaneda
#31947 THEME: Pinnacle – 1.3.7 closed live themetracbot
#33856 THEME: Pinnacle – 1.3.8 closed live themetracbot
#33981 THEME: Pinnacle – 1.3.9 closed live themetracbot
#34678 THEME: Pinnacle – 1.4.0 closed live themetracbot
#34793 THEME: Pinnacle – 1.4.1 closed live themetracbot
#34839 THEME: Pinnacle – 1.4.2 closed live themetracbot
#35151 THEME: Pinnacle – 1.4.3 closed live themetracbot
#35258 THEME: Pinnacle – 1.4.4 closed live themetracbot
#36876 THEME: Pinnacle – 1.4.5 closed live themetracbot
#37644 THEME: Pinnacle – 1.4.6 closed live themetracbot
#41193 THEME: Pinnacle – 1.4.7 closed live themetracbot
#41321 THEME: Pinnacle – 1.4.8 closed live themetracbot
#41666 THEME: Pinnacle – 1.5.0 closed live themetracbot
#41684 THEME: Pinnacle – 1.5.1 closed live themetracbot
#41795 THEME: Pinnacle – 1.5.2 closed live themetracbot
#41889 THEME: Pinnacle – 1.5.3 closed live Otto42
#41978 THEME: Pinnacle – 1.5.4 closed live themetracbot
#42208 THEME: Pinnacle – 1.5.5 closed live themetracbot
#43384 THEME: Pinnacle – 1.5.6 closed live themetracbot
#44286 THEME: Pinnacle – 1.5.7 closed live themetracbot
#44483 THEME: Pinnacle – 1.5.8 closed live themetracbot
#49080 THEME: Pinnacle – 1.5.9 closed live themetracbot


https://themes.svn.wordpress.org/pinnacle/1.2.0/screenshot.png

#89 @britner
3 years ago

Just updated a couple things for woocommerce 2.3 and put the redux css validation in.

  • Update: CSS validation to redux.
  • Update: Woocommerce 2.3 support.
  • Add: quantity input buttons back. (woocommerce removed in 2.3).

Please let me know if I can help move this review along.

Ben

#90 @grapplerulrich
3 years ago

  • Status changed from reviewing to approved

It looks good. APPROVED

#91 @karmatosed
3 years ago

  • Resolution set to live
  • Status changed from approved to closed

Congratulations, this theme is now live!

Note: See TracTickets for help on using tickets.