#22691 closed theme (not-approved)
THEME: HappenStance - 1.0.2
| Reported by: |
tomastoman
|
Owned by: |
chellycat
|
|---|---|---|---|
| Priority: | previously reviewed | Keywords: | theme-happenstance |
| Cc: | webdesign@… |
Description
HappenStance - 1.0.2
HappenStance is an easily customizable theme which can be used for your Blog, Magazine, Business, eCommerce or Events website. It is a fully responsive and Retina ready theme that allows for easy viewing on any device. HappenStance theme offers 4 pre-defined color schemes, 220 Google fonts, Theme Options panel for easy adaptation to your needs, selection between Boxed or Wide layout, 2 blog layouts (One Column and Grid - Masonry), contact information in header, ability to set your header logo and favicon, sidebar and footer widget areas, theme documentation and much more. Supports the popular plugins WooCommerce, The Events Calendar and Breadcrumb NavXT. Available in Czech, English, French, German, Russian, Slovak and Spanish.
Theme URL - http://www.tomastoman.cz/happenstance/
Author URL - http://www.tomastoman.cz/
SVN - https://themes.svn.wordpress.org/happenstance/1.0.2
ZIP - https://wordpress.org/themes/download/happenstance.1.0.2.zip?nostats=1
Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=/happenstance/1.0.1&new_path=/happenstance/1.0.2
History:
Change History (8)
This ticket was mentioned in Slack in #themereview by chellycat. View the logs.
10 years ago
@
#4
@
10 years ago
Thanks @emiluzelac for assigning me to this ticket. I'll have notes this weekend. :)
#5
@
10 years ago
- Resolution set to not-approved
- Status changed from reviewing to closed
Thanks for your patience. Here are my notes. Required fixes first, followed by recommendations. Let me know if you have questions.
REQUIRED:
- header.php, line 40: esc_attr() should only be used for escaping attributes inside tags. Use esc_html() to escape regular text. The same applies to lines 43, 46, and 49 in header.php. There may be others as well, please check the rest of the theme.
- headerdata.php, line 100: use esc_textarea() instead since this value came from a text area.
- The header menu (style.css starting on line 397) from http://www.cssplay.co.uk/menus/final_drop3.html is not GPL compatible, unfortunately. This is because the author has the following in his copyright info:
If you are using this on a personal web site then please add a link back to CSSplay and retain any copyright comment in the stylesheet.
If you are using this on a commercial web site, or as a paying job for a client, then please email me asking for permission - stu{at}cssplay.co.uk and in this case a donation to the 'Support CSSplay' fund is required.
Please replace this menu with another solution. For examples, I suggest checking out the menu in Underscores (https://github.com/Automattic/_s/) or any default theme.
- Post Formats can’t be premium because they’re a WordPress core feature. Core features can’t be restricted behind a pay wall.
- In addition, please add support for changing the header text color. This is a core feature that can’t be restricted to the premium version (if the premium version doesn’t have header text color then ignore this).
- functions.php, line 59 - wrap the add_theme_support() for Woo Commerce in if_function_exists(). I’d move it to a separate function.
RECOMMENDATIONS:
- The options page is confusing when there are premium-only features throughout. For the free version, I strongly suggest removing premium options from the options page.
- Similarly, I suggest removing premium features from the documentation to make it easier to scan and more relevant to the free theme. Alternatively, move all premium features to a “premium” section at the bottom of the documentation.
#6
follow-up:
↓ 7
@
10 years ago
Thanks for your review! I am working on the new update, but before I submit it, I would like to ask a couple of questions for clarification:
- headerdata.php, line 100: According to the Codex page, it seems that the esc_textarea() should be used for encoding a text inside a <textarea> element, so it probably would be better to keep esc_attr() in this case?
- functions.php, line 59: Will be sufficient if I keep the
add_theme_support( 'woocommerce' );within the happenstance_setup() function and use the following condition please?
if ( in_array( 'woocommerce/woocommerce.php', apply_filters( 'active_plugins', get_option( 'active_plugins' ) ) ) ) {
add_theme_support( 'woocommerce' );
}
Also, the Premium version does not use the core <?php get_header_textcolor(); ?>, there are only some custom functions for color settings.
Best regards,
Tomas Toman
#7
in reply to:
↑ 6
@
10 years ago
Replying to tomastoman:
- headerdata.php, line 100: According to the Codex page, it seems that the esc_textarea() should be used for encoding a text inside a <textarea> element, so it probably would be better to keep esc_attr() in this case?
Looks like you're right about esc_textarea(). Thanks for pointing that out! However I don't think esc_attr() works here because that's intended for use inside HTML element attributes, and this custom CSS is printed inside the style tags, and it's not an attribute. I suggest esc_html() instead.
- functions.php, line 59: Will be sufficient if I keep the
add_theme_support( 'woocommerce' );within the happenstance_setup() function and use the following condition please?if ( in_array( 'woocommerce/woocommerce.php', apply_filters( 'active_plugins', get_option( 'active_plugins' ) ) ) ) { add_theme_support( 'woocommerce' ); }
This should be fine!
Also, the Premium version does not use the core
<?php get_header_textcolor(); ?>, there are only some custom functions for color settings.
That's fine, but as per the guidelines, themes are required to use core functions for these features. The ability to change header text is a built-in WordPress feature, so if you plan to offer this feature, please use the core function instead of a custom function. Core features can't be hidden behind a pay wall.
One more note regarding the Custom CSS option. I thought some more about it and I feel that it's not necessary to include a CSS editor in your theme. We really like to encourage users to make style changes through a child theme. This way, if the parent theme is updated in the future, their customizations are unaffected. I would also argue that Custom CSS is closer to being plugin territory (there are quite a few Custom CSS plugins in the repository: https://wordpress.org/plugins/search.php?q=custom+css). Using a Custom CSS plugin gives users the ability to customize the CSS of any theme.
This isn't a requirement, and you can keep the CSS editor if you like. It's just something to think about for the future.
#8
@
10 years ago
Thank you for your replies! I just have uploaded the updated version. Here is the new ticket.
I decided to use my own CSS for the drop-down submenus. I have kept the same layout, but I used different CSS properties to achieve it.
I will consider removing the Custom CSS option for the future updates. You are right that this option is not necessary as it is possible to make style changes through a child theme.
I hope that I have fixed all the required issues.
Best regards,
Tomas Toman
Just a note that I am reviewing this theme currently (in case someone gets randomly assigned). I don't have permission to assign this ticket to myself, so I've asked the admins to do so. In the meantime I'm continuing with the review.