WordPress.org

Make WordPress Themes

Opened 4 years ago

Closed 4 years ago

#19058 closed theme (live)

THEME: Tally Framework - 0.7.7

Reported by: tallythemes Owned by: imon Hasan
Priority: previously reviewed Keywords: theme-tally-framework
Cc: admin@…

Attachments (1)

sc1.png (203.1 KB) - added by imon Hasan 4 years ago.

Download all attachments as: .zip

Change History (37)

#1 @themetracbot
4 years ago

  • Summary changed from THEME: Tally Framework - 0.6 to THEME: Tally Framework - 0.7

#2 @tallythemes
4 years ago

Update the Option Tree plugin and did the require changes

#3 @imon Hasan
4 years ago

  • Owner set to imon Hasan
  • Status changed from new to reviewing

#4 @themetracbot
4 years ago

  • Summary changed from THEME: Tally Framework - 0.7 to THEME: Tally Framework - 0.7.1

#5 @tallythemes
4 years ago

Hello imon Hasan,

When you will review my theme?

Thanks

#6 @imon Hasan
4 years ago

Theme Check pass

code check :

Require Issue

  • Themes are required to enqueue all stylesheets and scripts, using wp_enqueue_style()/wp_enqueue_script(), and hooked into an appropriate hook via callback function, rather than hard-coding stylesheet/script links or tags in the template.

Read this: http://justintadlock.com/archives/2010/11/17/how-to-load-files-within-wordpress-themes

you can not use include

  • No escaping for
/*
 	Footer Copyright
--------------------------------------------*/
if('tally_do_footer_copyright'):
	function tally_do_footer_copyright(){
		if(tally_is_footer() == 'none') return;
		
        echo '<div class="copy_text">';
			echo apply_filters('footer_copyright_text', tally_option('footer_copyright', 'Powered by <a href="http://wordpress.org/" target="_blank" title="A Semantic Personal Publishing Platform" rel="generator">WordPress</a> | Design by <a href="http://tallythemes.com/" target="_blank" rel="designer">TallyThemes</a>'));
        echo '</div>';
	}
	add_action('tally_footer', 'tally_do_footer_copyright', 10);
endif;

footer.php: Credit link url to http://tallythemes.com/ not escaped.

use esc_url() for all the link

read http://codex.wordpress.org/Validating_Sanitizing_and_Escaping_User_Data

unit test :

  • overflow in Post Format: Video (WordPress.tv)

check screenshot

pls check theme with http://codex.wordpress.org/Theme_Unit_Test


@imon Hasan
4 years ago

#7 @tallythemes
4 years ago

Hello Thanks for your feedback. I will fix the video issue and use esc_url() for all the link.

I used wp_enqueue_style()/wp_enqueue_script() to load all CSS and JS files. Can you please point me the hard-coding stylesheet/script links.

For include() issue. I created some function to load templates parts or files, It is a Framework Theme and I needed that includes in the functions.

Please let me know your opinion about include() issue

Thanks

#8 @themetracbot
4 years ago

  • Summary changed from THEME: Tally Framework - 0.7.1 to THEME: Tally Framework - 0.7.2

#9 @tallythemes
4 years ago

Hello Imon Hasan,

I fixed the video overflow issue and added esc_url() in footer links.

For include() issue. You are telling that "I can not use include" But I think I used include() wisely. I saw many approved wordpress themes are using include() to load functions file. even if you check your theme "hathor" in functions.php at line 496 include(get_template_directory() . '/lib/widgets.php'); you used include() you used include and require in you theme. I think you used it wisely. So why I can not use include()

Thanks

#10 @imon Hasan
4 years ago

I think their was misunderstanding
you can not use include() for load js & CSS .

but load php file with include () is OK.

thanks

#11 follow-up: @tallythemes
4 years ago

Hi,

Thanks for the clarification. I think I did not use include to load JS / CSS files. Can you please show me the code or the file name.

Thanks

#12 in reply to: ↑ 11 @imon Hasan
4 years ago

Replying to tallythemes:

Hi,

Thanks for the clarification. I think I did not use include to load JS / CSS files. Can you please show me the code or the file name.

Thanks

sorry their is a misunderstanding

#13 @imon Hasan
4 years ago

  • Status changed from reviewing to approved

no major issue found

#15 @tallythemes
4 years ago

Hi,

When the theme will live?

Thanks

#16 @chipbennett
4 years ago

  • Status changed from approved to reopened

#17 @chipbennett
4 years ago

  • Status changed from reopened to reviewing

Required

  • Functional output in functions.php must be inside a callback, hooked into an appropriate action
  • Custom filters must be properly namespaced (e.g. tally_html_footer instead of html_footer)
  • Do not display "default" logos
  • page-blog.php appears to be a blog posts template. The correct template file to use for the blog posts index, per the Template Hierarchy, is home.php.

Note

  • Loader functions, e.g. function tally_file_dri(), appear to be essentially equivalent to locate_template()?

#18 @themetracbot
4 years ago

  • Summary changed from THEME: Tally Framework - 0.7.2 to THEME: Tally Framework - 0.7.3

#19 follow-up: @tallythemes
4 years ago

Hello

I have updated the theme. With some improvement and the require changes.

All functions are not loaded vis after_setup_theme hook

Fixed namespaced issue

Removed default logo not title will display

page-blog.php is an additional page template for blog. index.php file to use for the blog posts index

tally_file_dri() used to load some files and it have child theme support like tally_file_dri()

Thanks

#20 @themetracbot
4 years ago

  • Summary changed from THEME: Tally Framework - 0.7.3 to THEME: Tally Framework - 0.7.4

#21 in reply to: ↑ 19 @chipbennett
4 years ago

Replying to tallythemes:

page-blog.php is an additional page template for blog. index.php file to use for the blog posts index

No, that's not correct. Per the Template Hierarchy, index.php is the default fallback template for all contexts. The blog posts index template file is home.php.

Please read here for more detailed information:
http://www.chipbennett.net/2013/09/14/home-page-and-front-page-and-templates-oh-my/

#22 @tallythemes
4 years ago

Hi,

I know that home.php is for display posts index. I am taking about my theme, it's don't need home.php, it's index.php will display blog posts index. if you check "twentyfourteen" theme you will see there is no home.php file and index.php is showing blog post's index.

Now please tell me what I need to do to approve the theme?

Thanks

#23 @chipbennett
4 years ago

Twenty Fourteen allows the blog posts index to fallback to index.php, and does not include page-blog.php. That's the difference.

Having page-blog.php implies that the Theme has defined a custom output intended for the blog posts index; as such, that custom output must be implemented as the Template Hierarchy-defined home.php template file, and not as a page-blog.php custom page template.

You either need to re-implement page-blog.php as home.php, or else remove it.

#24 @themetracbot
4 years ago

  • Summary changed from THEME: Tally Framework - 0.7.4 to THEME: Tally Framework - 0.7.5

#25 @tallythemes
4 years ago

Just updated theme with home.php file

#26 @chipbennett
4 years ago

page-blog.php is still in the Theme. It needs to be removed. You've added a pointless home.php file that acts no differently from index.php.

The problem wasn't the omission of home.php, but rather the inclusion of page-blog.php.

#27 follow-up: @tallythemes
4 years ago

Hi,

page-blog.php and home.php are displaying same content. I have added page-blog.php with this thinking that someone want to create a front page with a static page and want to have a blog with a name for example "Our Blog".

I think you are expert, so please tell me am I still need to remove page-blog.php file. If you say delete. I will delete it.

Thanks

#28 in reply to: ↑ 27 @chipbennett
4 years ago

Replying to tallythemes:

page-blog.php and home.php are displaying same content. I have added page-blog.php with this thinking that someone want to create a front page with a static page and want to have a blog with a name for example "Our Blog".

That is the purpose of home.php.

Please read here for more detailed information:
http://www.chipbennett.net/2013/09/14/home-page-and-front-page-and-templates-oh-my/

#29 @themetracbot
4 years ago

  • Summary changed from THEME: Tally Framework - 0.7.5 to THEME: Tally Framework - 0.7.6

#30 @tallythemes
4 years ago

just removed page-blog.php and home.php form the theme.

#31 @imon Hasan
4 years ago

@ chip i checked all issue solved

Require

  • you need to declare image licence in you readme.txt

thanks

#32 @imon Hasan
4 years ago

another one is theme get a fix favicon and which is not changeable from theme option

pls remove it

thanks

#33 @themetracbot
4 years ago

  • Summary changed from THEME: Tally Framework - 0.7.6 to THEME: Tally Framework - 0.7.7

#34 @tallythemes
4 years ago

@ imon Hasan
I removed the favicon icon from the theme. Also I update the readme.txt for the image licence .

Thanks

#35 @imon Hasan
4 years ago

  • Status changed from reviewing to approved

no major issue


#36 @chipbennett
4 years ago

  • Resolution set to live
  • Status changed from approved to closed
Note: See TracTickets for help on using tickets.