WordPress.org

Make WordPress Themes

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#3029 closed theme (not-approved)

THEME: Suffusion - 3.7.4

Reported by: sayontan Owned by: emiluzelac
Priority: Keywords: theme-suffusion
Cc: sayontan@…

Description

Suffusion - 3.7.4

An elegant, versatile and browser-safe theme with a power-packed set of options. It has 19 widget areas, one-column, two-column and three-column fixed-width and flexible-width formats, 10 pre-defined templates, 17 pre-defined color schemes, two customizable multi-level drop-down menus, featured posts, a magazine layout, tabbed sidebars, widgets for Twitter, Social Networks and Google Translator, translations in many languages and RTL language support. WP 3.0 Menus, Custom Post Types and Custom Taxonomies are integrated. A BuddyPress support pack is available as a plugin for smooth BuddyPress integration. Support forum at http://www.aquoid.com/forum.

Theme URL - http://www.aquoid.com/news/themes/suffusion/
Author URL - http://mynethome.net/blog

SVN - http://themes.svn.wordpress.org/suffusion/3.7.4
ZIP - http://wordpress.org/extend/themes/download/suffusion.3.7.4.zip?nostats=1

Diff with previous version: http://themes.trac.wordpress.org/changeset?old_path=/suffusion/3.7.3&new_path=/suffusion/3.7.4

All previous tickets for this theme: http://themes.trac.wordpress.org/query?col=id&col=summary&col=keywords&col=owner&col=status&col=resolution&keywords=~theme-suffusion&order=priority

https://themes.svn.wordpress.org/suffusion/3.7.4/screenshot.png

Change History (14)

#1 @sayontan
6 years ago

There are quite a few changes in this release. Primarily:

  1. Switch to Settings API
  2. Removal of TimThumb
  3. Removal of all fopen calls (which required me to find workarounds)

Please let me know in case of issues.

#2 @emiluzelac
6 years ago

  • Owner set to emiluzelac
  • Status changed from new to accepted

#3 @emiluzelac
6 years ago

  • Resolution set to not-approved
  • Status changed from accepted to closed

NOTE:
Since there are many changes to your theme the review beyond Diff was necessary. Please make a note of that.

Theme Check

  • REQUIRED: get_users_of_blog found in the file authors.php. Deprecated since version 3.1. Use get_users() instead.
Line 32: $authors = get_users_of_blog();

There are some issues with the provided version:

JS Errors

Webpage error details

User Agent: Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.1; Trident/4.0; GTB6.6; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; .NET4.0C; Tablet PC 2.0)
Timestamp: Mon, 21 Feb 2011 04:55:30 UTC
Message: 'suf_featured_fx' is undefined
Line: 247
Char: 2
Code: 0
URI: /wp-content/themes/suffusion/scripts/suffusion.js?ver=3.7.4

That refers to:

	// Featured content slider
	$j('#sliderContent').cycle({

There's a improper usage of "wp_enqueue" and this creates an error before your login to WordPress. See: wp_enqueue examples and documentation.

Webpage error details

User Agent: Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.1; Trident/4.0; GTB6.6; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; .NET4.0C; Tablet PC 2.0)
Timestamp: Mon, 21 Feb 2011 05:32:37 UTC
Message: 'suf_jq_masonry_enabled' is undefined
Line: 220
Char: 2
Code: 0
URI: /wp-content/themes/suffusion/scripts/suffusion.js?ver=3.7.4

Theme Options

Suffusion Options do not work well in IE8. The initial page is fine, however after you click on i.e. the page becomes unresponsive and there is a pop-up error as well. See: screenshot

In first page of your Suffusion Option you have an image that's not compatible with GPL. This image Don't Panic is a copyrighted material of Touchstone Pictures'/Spyglass Entertainment's THE HITCHHIKER'S GUIDE TO THE GALAXY. See: http://en.wikipedia.org/wiki/Touchstone_Pictures

Post Format Test: Video

Embedded Video exceeds design content. See: screenshot

Clearing Floats

While testing the images the floats are not cleared. See: screenshot

Coding

You have way to much inline styles in your codes, please try to keep this at minimum, or incorporate in i.e. style.css. See: pastebin please.

Custom Menu

Custom menu should allow users to remove/add anything they wish from this area. This theme does not really allow that. When new menu is created and when desired pages are chosen, they are being simply added to "Pages", "Categories" instead of providing full control over the menu. See screenshot.

Theme Screenshot

The screenshot you supplied does not match your current design. Please change the screenshot so that it reflects the current version.

In most cases when minor and other changes are made from the previously approved version and if there were no notable/major issues, the theme would be accepted and approved. Even if we only make notes about conflicts or errors and place that aside, the theme would not be approved because of copyright violations.

Please correct the above mention issues and resubmit your theme. If you have any questions, please do not hesitate to post them here.

Sorry to say but this theme update is not accepted, therefore cannot be approved.

Helpful Resources

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

#4 @sayontan
6 years ago

Emil,
Thanks for the review. I will fix the errors. However I believe the following are not errors:

REQUIRED: get_users_of_blog found in the file authors.php. Deprecated since version 3.1. Use get_users() instead.

The get_users() function is available only in 3.1, which is not yet live. So I cannot exclude the call to get_users_of_blog.

You have way to much inline styles in your codes, please try to keep this at minimum, or incorporate in i.e. style.css.
I thought inline styles refer to the "style" attribute in an HTML element. Nonetheless I understand what you are trying to say. However, for that I will refer you to this long discussion I had on the reviewers' list: http://lists.wordpress.org/pipermail/theme-reviewers/2010-December/thread.html#3187. Basically this is all one long generated string based on the options you set in the theme. So there is no way I can pre-write them to a file. This is in accordance to the suggestion provided by Otto and Michael Fields. See here: http://lists.wordpress.org/pipermail/theme-reviewers/2010-December/003203.html. Moreover note that the reviewers have outlawed using fopen, so that is out of the question. As an alternative, you can set an option in the theme to hook this through a template_redirect, so this is not printed in the theme. It is just not the default option.

Custom menu should allow users to remove/add anything they wish from this area. This theme does not really allow that. When new menu is created and when desired pages are chosen, they are being simply added to "Pages", "Categories" instead of providing full control over the menu.
Well, this is supplemental functionality over and above what Custom Menus provide. The Pages and Categories that you mention are very easily disabled from the Options menu (Other Graphical Elements -> Main Navigation Bar). Mind you, these are needed, because of specific use cases. Basically you cannot replicate this with custom menus: dynamically add all pages and sub-pages to the menu (custom menus only let you add top level pages automatically not sub-pages). Ditto for categories. So I don't see why this is being called out as an issue. You can still do what you please with your menus and all of it will be reflected in the output.

The screenshot you supplied does not match your current design. Please change the screenshot so that it reflects the current version.
This I don't understand. How is the screenshot different? The only difference in the screenshot and the default layout is that the screenshot shows what you see when you hover over a menu item - that's all.

#5 @emiluzelac
6 years ago

Hi,

Required

The required is recommended in this case, however I believe that's better to fix this before 3.1 is out. And as you're are aware that 3.1 is around the corner.

  • REQUIRED: get_users_of_blog found in the file authors.php. Deprecated since version 3.1. Use get_users() instead.

Inline Styles

This was recommendation, sorry for not marking that properly. I should know better than that. Regardless of the above mentioned discussion (and now marking this as recommended) I still think that inline styles a/k/a embedded css codes should be at the lowest amount.

Inline styles (as used in your theme) will always have the highest precedence which means that user will have very hard time overwriting those styles no matter what they do and yes in some cases you could use ( !important ) but that doesn't work all the time because we cannot style pseudo-elements and -classes with having this in theme.

Once again, it's not banned, therefore you can use if there's no other alternatives, but with that much of inline styles, the design cannot be called "clean" :)

Another example is TwentyTen theme and removal of inline styles generated by gallery:

/**
 * Remove inline styles printed when the gallery shortcode is used.
 *
 * Galleries are styled by the theme in Twenty Ten's style.css.
 *
 * @since Twenty Ten 1.0
 * @return string The gallery style filter, with the styles themselves removed.
 */
function twentyten_remove_gallery_css( $css ) {
	return preg_replace( "#<style type='text/css'>(.*?)</style>#s", '', $css );
}
add_filter( 'gallery_style', 'twentyten_remove_gallery_css' );

Even they're fighting this and the inline styles are only few line of codes.

Screenshot (screenshot.png)
Take a look at this screenshot the sidebars don't match ;)

Custom Menu

The menu does allow you to keep or to remove anything you want. Menu also allow you to have a standard dropdown, including the subpages as well. Whatever you can do with CSS, menu will support.

So yes, if user want to remove all* and add it's own menu, excluding the "pages" & "categories" (within custom menu) we should give them that option. See: screenshot which shows that subpages can indeed be added to custom menu :)

Other notes: If user wants to add "Home" to this theme's menu, the "Home" link will not appear at all. Here's the screenshot for that as well.

This refers to:

'fallback_cb' => 'display_home'

And (just as an example) from TwentyTen

/**
 * Get our wp_nav_menu() fallback, wp_page_menu(), to show a home link.
 *
 * To override this in a child theme, remove the filter and optionally add
 * your own function tied to the wp_page_menu_args filter hook.
 *
 * @since Twenty Ten 1.0
 */
function twentyten_page_menu_args( $args ) {
	$args['show_home'] = true;
	return $args;
}
add_filter( 'wp_page_menu_args', 'twentyten_page_menu_args' );

PS Earlier, I wasn't talking about design/graphic, but about the menu functionalities itself :)

Please see http://codex.wordpress.org/Function_Reference/wp_nav_menu

#6 @sayontan
6 years ago

Emil,
Perhaps I should have made myself more clear regarding menus.
Basically you cannot replicate this with custom menus: dynamically add all pages and sub-pages to the menu (custom menus only let you add top level pages automatically not sub-pages).

Here is what I meant. I have users who do this:

  1. Create a page/sub-page (particularly in multi-author sites).
  2. Expect that the newly created page automatically shows up in the navigation menu, without having to add it anywhere. Note that if you select the "Automatically add new top-level pages" checkbox in the menu page, this automatically adds new top level pages, not sub-pages.
  3. Another use case is where people want all added pages to show up in the menu, except a chosen few.

I don't think these are possible in the current menu framework provided by WP. It is here that my menus supplement what WP provides. You can still continue adding things to custom menus - I have neither broken nor bypassed that functionality. I have only provided something extra to help users achieve this. This has nothing to do with CSS at all.

Regarding "inline styles:
Inline styles (as used in your theme) will always have the highest precedence which means that user will have very hard time overwriting those styles no matter what they do

This would be correct if I was using inline styles. I am not!! Inline styles are things you embed in an HTML element. E.g. <div style="width: 400px"> would be an inline style. I am not doing anything of this sort. I am simply printing plain CSS rules on the HTML page (just like the way the admin-bar CSS is printed in HTML in version 3.1). You can very easily override this, both within the theme (there is an option for Custom Includes in "Back-End Settings") and beyond (using a higher specificity in a child theme). Just take a look at the very first printed style there:

.page-template-1l-sidebar-php #wrapper{width:1000px;max-width:1000px;min-width:1000px;}

I don't need !important to override this. I could simply define a new style with this:

.page-template-1l-sidebar-php div#wrapper{width:1000px;max-width:1000px;min-width:1000px;}

Even if I were to include these as an external file, the same rules would be present and would be overridden in the same way. So all that is happening here is that the CSS is being printed in HTML as opposed to being linked through a file.

Plus I have provided alternatives: if you go to Back-end Settings -> Site Optimization you will be able to include the CSS as a link instead of printing it to the HTML (which is what you are asking for, I guess).

#7 follow-up: @sayontan
6 years ago

Emil,
Quick question. If I resubmit the theme will I be relegated to the bottom of the queue?

Sayontan.

#8 @emiluzelac
6 years ago

Basically you cannot replicate this with custom menus: dynamically add all pages and sub-pages to the menu (custom menus only let you add top level pages automatically not sub-pages).


When we have a custom menu, newly created page will not be automatically included, unless user includes this manually. So if user wants the default option, he or she would not require custom menu at all. That's why we need to be sure that custom menu has all of the functionalities and options for others who desire to use custom menu and create their very own navigation from "Menus".

If we limit users to what they can or cannot do, that will be called "dictating" and we don't want that. In my personal opinion options are always a big plus and I am sure that you would agree to that.

Here is what I meant. I have users who do this:

1.Create a page/sub-page (particularly in multi-author sites).
2.Expect that the newly created page automatically shows up in the navigation menu, without having to add it anywhere. Note that if you select the "Automatically add new top-level pages" checkbox in the menu page, this automatically adds new top level pages, not sub-pages.
3.Another use case is where people want all added pages to show up in the menu, except a chosen few.


  1. That can be done via Menus as well.
  2. Pages/Sub-Pages are possible via custom menus too.
  3. If they're using Menus, it's possible to include/exclude anything they need

Basically what I am trying to say here is that if the theme is using the custom menu support, menu should have all options for users who want to create custom navigation. And for anyone that doesn't, they would not even use this feature.

However if they don't have an option to remove "Pages" & Categories while using the custom menu, than we're dictating and telling them what they can or cannot do.


When this is present in HTML:

<style type="text/css">
</style>

auto-generated or hard-coded are embedded styles :) Anyways, this isn't required, feel free to skip that completely.

Thanks

#9 in reply to: ↑ 7 @emiluzelac
6 years ago

Replying to sayontan:

Emil,
Quick question. If I resubmit the theme will I be relegated to the bottom of the queue?

Sayontan.

This was a previously approved theme, I don't believe that it will go outside of "Priority 1" ticket queue :)

#10 @sayontan
6 years ago

Emil,
I believe there is a disconnect here:

If we limit users to what they can or cannot do, that will be called "dictating" and we don't want that. In my personal opinion options are always a big plus and I am sure that you would agree to that.

I have not limited any functionality. You are free to use any custom menus you want in whatever manner you want. If you want to disable Pages and Categories, you can do it simply by switching off 2 options (as I said, you can remove them from Other Graphical Elements -> Main Navigation Bar, by excluding all pages and categories). Then you will not see the "Pages" and "Categories" tabs and all you will see are the Custom Menus in whichever way you have set them up.

1. That can be done via Menus as well.

  1. Pages/Sub?-Pages are possible via custom menus too.
  2. If they're using Menus, it's possible to include/exclude anything they need

I don't think so. I don't believe you can replicate the cases I have stated without explicitly adding pages to the menu every time you create a sub-page.

Anyway, I am simply turning off the options by default, so you will not see them on the default screen. Users who want to use the features can always do so.

#11 @emiluzelac
6 years ago

Thanks for clearing that out.

#12 @sayontan
6 years ago

Emil,
I have uploaded a new version: http://themes.trac.wordpress.org/ticket/3036.

Please note that you will still get an error about "get_usrs_for_blog()", however if you look at the code you will see that there is a "function_exists" check for "get_users()" over there.

#13 @sayontan
6 years ago

... And unfortunately that has got relegated!

#14 @emiluzelac
6 years ago

I will review tomorrow.

Note: See TracTickets for help on using tickets.