WordPress.org

Make WordPress Themes

Opened 7 months ago

Closed 7 months ago

Last modified 7 months ago

#14404 closed theme (not-approved)

THEME: raven - 1.0.1

Reported by: tskk Owned by: greenshady
Priority: new theme Keywords: theme-raven
Cc: tskk79@…

Description

raven - 1.0.0

Responsive WordPress magazine theme with multiple home page layouts, 300x250 ads, 125x125 ads, 2 premade ready to use color schemes/skins, 2 page layouts including a full width page template, featured posts, social icons, threaded comments and widget support.

Theme URL -
Author URL - http://www.themealley.com/

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

History:

Ticket Summary Status Resolution Owner
#14404 THEME: raven - 1.0.1 closed not-approved greenshady

(this ticket)

#14458 THEME: raven - 1.0.2 closed live ZGani
#14653 THEME: raven - 1.0.3.1 closed live emiluzelac
#14806 THEME: raven - 1.0.4 closed live ZGani
#14839 THEME: raven - 1.0.5 closed live emiluzelac
#14969 THEME: raven - 1.0.6 closed live rohitink
#15156 THEME: raven - 1.0.7 closed live rohitink
#15257 THEME: raven - 1.0.7.1 closed live rohitink
#15362 THEME: raven - 1.0.8 closed live chipbennett
#15489 THEME: raven - 1.0.9 closed live emiluzelac
#15691 THEME: raven - 1.0.10.1 closed live rohitink
#15813 THEME: raven - 1.0.11 closed live rohitink
#16062 THEME: raven - 1.0.12 closed live Frumph
#16145 THEME: raven - 1.0.13 closed live rohitink
#16266 THEME: raven - 1.0.14 closed live rohitink
#16587 THEME: raven - 1.0.15 closed live rohitink
#16830 THEME: raven - 1.0.17 closed live robin90
#17001 THEME: raven - 1.0.18 closed live robin90
#17194 THEME: raven - 1.0.19 closed live robin90
#17367 THEME: raven - 1.0.20 closed live robin90
#17488 THEME: raven - 1.0.21 closed live rohitink
#17679 THEME: raven - 1.0.22 closed live robin90
#17801 THEME: raven - 1.0.23 closed live catchthemes
#18079 THEME: raven - 1.0.24 closed live robin90


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

Change History (9)

comment:1 themetracbot7 months ago

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

raven - 1.0.1

Responsive WordPress magazine theme with multiple home page layouts, 300x250 ads, 125x125 ads, 2 premade ready to use color schemes/skins, 2 page layouts including a full width page template, featured posts, social icons, threaded comments and widget support.

Theme URL -
Author URL - http://www.themealley.com/

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

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

History:

Ticket Summary Status Resolution Owner
#14404 THEME: raven - 1.0.1 closed not-approved greenshady

(this ticket)

#14458 THEME: raven - 1.0.2 closed live ZGani
#14653 THEME: raven - 1.0.3.1 closed live emiluzelac
#14806 THEME: raven - 1.0.4 closed live ZGani
#14839 THEME: raven - 1.0.5 closed live emiluzelac
#14969 THEME: raven - 1.0.6 closed live rohitink
#15156 THEME: raven - 1.0.7 closed live rohitink
#15257 THEME: raven - 1.0.7.1 closed live rohitink
#15362 THEME: raven - 1.0.8 closed live chipbennett
#15489 THEME: raven - 1.0.9 closed live emiluzelac
#15691 THEME: raven - 1.0.10.1 closed live rohitink
#15813 THEME: raven - 1.0.11 closed live rohitink
#16062 THEME: raven - 1.0.12 closed live Frumph
#16145 THEME: raven - 1.0.13 closed live rohitink
#16266 THEME: raven - 1.0.14 closed live rohitink
#16587 THEME: raven - 1.0.15 closed live rohitink
#16830 THEME: raven - 1.0.17 closed live robin90
#17001 THEME: raven - 1.0.18 closed live robin90
#17194 THEME: raven - 1.0.19 closed live robin90
#17367 THEME: raven - 1.0.20 closed live robin90
#17488 THEME: raven - 1.0.21 closed live rohitink
#17679 THEME: raven - 1.0.22 closed live robin90
#17801 THEME: raven - 1.0.23 closed live catchthemes
#18079 THEME: raven - 1.0.24 closed live robin90


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

comment:2 greenshady7 months ago

  • Owner set to greenshady
  • Status changed from new to reviewing

comment:3 greenshady7 months ago

Sections marked with "(required)" are issues that you must address before inclusion the in theme directory. Other notes are tips and recommendations.

At this time, the theme cannot be approved, particularly because of no sanitize/validate functions against the widget data. Once you've corrected the required items, please upload a new copy of the theme.

Home/posts page (required)

There's no prev/next posts navigation. This is because, by default, you're loading the index-magelvn.php template, which doesn't show the nav. The posts page should always show the posts nav.

I recommend turning your index-magelvn.php template into a custom page template instead.

Archive titles (required)

For author and yearly archives, the title reads, "Archives for :". It doesn't show anything else.

For daily archives, the title reads, "Archives for : Month Year".

Monthly archives display correctly.

raven_get_page_id()

A tip: WordPress has a function that will return the ID for you on single page views: get_queried_object_id().

Adding styles/scripts

You don't need the !is_admin() checks before loading scripts/styles. Since you're hooking to wp_enqueue_scripts, it will never run in the admin anyway.

Registering widgets

There's no need for multiple functions hooked to widgets_init. You can wrap all your register_widget() calls in a single function.

I also don't think you need multiple ad widgets. You can combine those two widgets into one. Just make an option for the different sizes.

Sanitize widget data (required)

In raven_sidebarads::update() and raven_sidebarads_onetwofive::update(), you need to sanitize widget data (url and image options) before returning it.

Theme unit test data (required)

  • Definition lists need some work. There's no different between a <dt> and a <dd> in design. You can't tell what's a term and what's a definition.

Theme options page

Consider changing the colors on the page. It was extremely hard to read the text on this page.

comment:4 greenshady7 months ago

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

comment:5 tskk7 months ago

Thanks Justin for the review.
Is pagination absolutely required? I have standard blog layout as option which has pagination. Magazine style layouts don't have pagination and magelvn is semi magazine.

comment:6 greenshady7 months ago

It's just that it needs pagination by default. I don't see anything wrong with an option for the magazine layout taking over the front page (without pagination). You're more than welcome to ask about this on the mailing list though because I may be wrong, but I'm pretty sure that defaults would require the pagination.

My main concern is with sanitizing the widget data. Feel free to ask me any questions about getting that done.

comment:7 tskk7 months ago

Ok, I will add pagination, no biggie.

Regarding sanitizing ad widgets, i changed

	function update( $new_instance, $old_instance ) {
		$instance = $old_instance;

		$instance['image'] = $new_instance['image'];
		$instance['url'] = $new_instance['url'];

		return $instance;
	}

to

	function update( $new_instance, $old_instance ) {
		$instance = $old_instance;

		$instance['image'] = esc_url($new_instance['image']);
		$instance['url'] = esc_url($new_instance['url']);

		return $instance;
	}

Is that good enough?
Thanks for the help.

comment:8 greenshady7 months ago

esc_url() is really more of an output validation function rather than input.

For the image, I'd use sanitize_file_name(). That would make sure that only a valid file name was used.

For the URL, I'd go with sanitize_text_field() (you could go with extra stuff and validate it as a URL, but that's not necessary).

References:
http://codex.wordpress.org/Function_Reference/sanitize_file_name
http://codex.wordpress.org/Function_Reference/sanitize_text_field

Here's what I'm thinking would be optimal:

	function update( $new_instance, $old_instance ) {
		$instance = $old_instance;

		$instance['image'] = sanitize_text_field( $new_instance['image'] );
		$instance['url']   = sanitize_file_name(  $new_instance['url']   );

		return $instance;
	}

comment:9 tskk7 months ago

Thank Justin,
I fixed all required issues and some recommended issues and uploaded a new version.
http://themes.trac.wordpress.org/ticket/14458
The other recommended issues, I will fix later.

Note: See TracTickets for help on using tickets.