WordPress.org

Make WordPress Themes

Opened 2 years ago

Closed 19 months ago

#29217 closed theme (live)

THEME: Etrigan – 0.91

Reported by: rohitink Owned by: holger1411
Priority: new theme Keywords: theme-etrigan
Cc: rohit@…

Description

Etrigan - 0.9

Etrigan is a very Powerful MultiPurpose theme Built with Bootstrap Framework. It supports WooCommerce and comes integrated with the latest Font Awesome Icons. There are multiple ways to showcase your Featured Products and Posts. Sidebar is fully Customizable with a Beautiful Recent Post Widget. There are so many Features that you won’t wanna switch this theme and it also Comes with Multiple Color Options. Demo Here: http://demo.rohitink.com/etrigan/

Theme URL - https://rohitink.com/2015/12/15/etrigan-multipurpose-theme/
Author URL - http://rohitink.com

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

History:

Ticket Summary Status Resolution Owner
#29217 THEME: Etrigan – 0.91 closed live holger1411

(this ticket)

#33977 THEME: Etrigan – 1.0 closed live themetracbot
#34087 THEME: Etrigan – 1.0.1 closed live themetracbot
#34185 THEME: Etrigan – 1.0.1.1 closed live themetracbot
#34239 THEME: Etrigan – 1.0.1.2 closed live themetracbot
#34674 THEME: Etrigan – 1.0.1.3 closed live themetracbot
#37173 THEME: Etrigan – 1.0.1.5 closed live themetracbot
#38230 THEME: Etrigan – 1.0.1.6 closed live themetracbot
#38538 THEME: Etrigan – 1.0.2 closed live themetracbot
#39341 THEME: Etrigan – 1.0.3 closed live themetracbot
#42283 THEME: Etrigan – 1.0.4 closed live themetracbot
#46385 THEME: Etrigan – 1.0.5 closed live themetracbot
#46558 THEME: Etrigan – 1.0.6 closed live themetracbot


https://themes.svn.wordpress.org/etrigan/0.9/screenshot.png

Attachments (1)

screenshot-localhost 8888 2016-04-15 11-13-53.png (78.4 KB) - added by holger1411 22 months ago.
Ghost links appear here

Download all attachments as: .zip

Change History (26)

#1 @themetracbot
22 months ago

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

#2 @grapplerulrich
22 months ago

  • Owner npbintoflash deleted

I am sorry this review is taking so long. Sometimes people are unable to carry on the review, this may have happened this time. As a result, I am going to add this to the new queue again as a priority. This may mean it still takes time, but will prevent this ticket being held for so long by a reviewer that isn't able to carry on. Thanks for your patience.

If you are the reviewer and able to do this review, please carry on and request you get added back in Slack #themereview.

#3 @themetracbot
22 months ago

  • Owner set to tinkerbelly

#4 @grapplerulrich
22 months ago

  • Owner tinkerbelly deleted

I am sorry this review is taking so long. Sometimes people are unable to carry on the review, this may have happened this time. As a result, I am going to add this to the new queue again as a priority. This may mean it still takes time, but will prevent this ticket being held for so long by a reviewer that isn't able to carry on. Thanks for your patience.

If you are the reviewer and able to do this review, please carry on and request you get added back in Slack #themereview.

#5 @themetracbot
22 months ago

  • Owner set to holger1411

#6 @holger1411
22 months ago

No debug error, every asset was "enqueued" the right way. Unit tests looks good. Everything fine, BUT:

  • On the right hand side of the "home" link appear "ghost" links for all pages. Code looks like:

<li id="menu-item-174" class=""><a></a></li>
Seems to be a problem with the nav walker.

  • The build in search form is moved down and appears below the main menu but above the main content. Generates a ugly gap.

I test it on FF, Chrome and Safari on a Mac.

Please fix the layout problem and the empty links before I can approve it

#7 @holger1411
22 months ago

The image text ist wrong...sorry...seems the nav walker instead of the icons caused the problem. But use the image as reference to find the spot

#8 @rohitink
21 months ago

Hi,

Thanks for your review. I Will need 2-3 days to fix the theme.
I have found a few other issues on my own as well.
Once its all fixed, i will update the ticket :)

#9 @themetracbot
21 months ago

  • Summary changed from THEME: Etrigan – 0.9 to THEME: Etrigan – 0.91

Etrigan - 0.91

Etrigan is a very Powerful MultiPurpose theme Built with Bootstrap Framework. It supports WooCommerce and comes integrated with the latest Font Awesome Icons. There are multiple ways to showcase your Featured Products and Posts. Sidebar is fully Customizable with a Beautiful Recent Post Widget. There are so many Features that you won&#8217;t wanna switch this theme and it also Comes with Multiple Color Options. Demo Here: http://demo.rohitink.com/etrigan/

Theme URL - https://rohitink.com/2015/12/15/etrigan-multipurpose-theme/
Author URL - http://rohitink.com

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=etrigan/0.9&new_path=etrigan/0.91

History:

Ticket Summary Status Resolution Owner
#29217 THEME: Etrigan – 0.91 closed live holger1411

(this ticket)

#33977 THEME: Etrigan – 1.0 closed live themetracbot
#34087 THEME: Etrigan – 1.0.1 closed live themetracbot
#34185 THEME: Etrigan – 1.0.1.1 closed live themetracbot
#34239 THEME: Etrigan – 1.0.1.2 closed live themetracbot
#34674 THEME: Etrigan – 1.0.1.3 closed live themetracbot
#37173 THEME: Etrigan – 1.0.1.5 closed live themetracbot
#38230 THEME: Etrigan – 1.0.1.6 closed live themetracbot
#38538 THEME: Etrigan – 1.0.2 closed live themetracbot
#39341 THEME: Etrigan – 1.0.3 closed live themetracbot
#42283 THEME: Etrigan – 1.0.4 closed live themetracbot
#46385 THEME: Etrigan – 1.0.5 closed live themetracbot
#46558 THEME: Etrigan – 1.0.6 closed live themetracbot


https://themes.svn.wordpress.org/etrigan/0.91/screenshot.png

#10 @rohitink
21 months ago

Hi @holger1411

I have fixed a Numerous nos. of issues I found myself, and with the help of an admin review on my other theme.

However, the issue you reported related to ghost links was not found. Perhaps, there could be problem with the Test Envt you were using. Some dummy links were added by mistake in the panel. Anyway, I tested it completely on http://demo.rohitink.com/etrigan/ And No Ghost Links were found.

What you may see is perhaps an extra select box added at bottom to convert it into a mobile menu on smaller screens.

#11 @holger1411
21 months ago

Hi!

To see it do a clean WP install with just the sample content and/or the unit test dummy data.
https://codex.wordpress.org/Theme_Unit_Test

See´s that it just appears if the main menu isn´t (fully) in use. It renders empty link elements.

#12 @rohitink
21 months ago

Thanks, I have found the Issue. Now working on it.

#13 @themetracbot
21 months ago

  • Summary changed from THEME: Etrigan – 0.91 to THEME: Etrigan – 0.92

Etrigan - 0.92

Etrigan is a very Powerful MultiPurpose theme Built with Bootstrap Framework. It supports WooCommerce and comes integrated with the latest Font Awesome Icons. There are multiple ways to showcase your Featured Products and Posts. Sidebar is fully Customizable with a Beautiful Recent Post Widget. There are so many Features that you won&#8217;t wanna switch this theme and it also Comes with Multiple Color Options. Demo Here: http://demo.rohitink.com/etrigan/

Theme URL - https://rohitink.com/2015/12/15/etrigan-multipurpose-theme/
Author URL - http://rohitink.com

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=etrigan/0.91&new_path=etrigan/0.92

History:

Ticket Summary Status Resolution Owner
#29217 THEME: Etrigan – 0.91 closed live holger1411

(this ticket)

#33977 THEME: Etrigan – 1.0 closed live themetracbot
#34087 THEME: Etrigan – 1.0.1 closed live themetracbot
#34185 THEME: Etrigan – 1.0.1.1 closed live themetracbot
#34239 THEME: Etrigan – 1.0.1.2 closed live themetracbot
#34674 THEME: Etrigan – 1.0.1.3 closed live themetracbot
#37173 THEME: Etrigan – 1.0.1.5 closed live themetracbot
#38230 THEME: Etrigan – 1.0.1.6 closed live themetracbot
#38538 THEME: Etrigan – 1.0.2 closed live themetracbot
#39341 THEME: Etrigan – 1.0.3 closed live themetracbot
#42283 THEME: Etrigan – 1.0.4 closed live themetracbot
#46385 THEME: Etrigan – 1.0.5 closed live themetracbot
#46558 THEME: Etrigan – 1.0.6 closed live themetracbot


https://themes.svn.wordpress.org/etrigan/0.92/screenshot.png

#14 @rohitink
21 months ago

  • Summary changed from THEME: Etrigan – 0.92 to THEME: Etrigan – 0.91

Thanks @holger1411 for finding the error. I have that fixed :)

#15 @holger1411
21 months ago

  • Status changed from reviewing to approved

Thx! Looks good here now.
Will approve it.

#16 @jcastaneda
20 months ago

I'll take on the admin review of this one. I'll try to get to it be the end of the day :)

#17 @jcastaneda
20 months ago

  • Status changed from approved to reopened

Just a few things and we can get this live. :)

  • please use wp_add_inline_style for inc/css-mods.php
  • validation on posts widget. It's great you are using strip_tags but you do have to remember that in the update() method you may also need to validate the input. Using is_integer(), is_boolean() can be used to make sure you are getting and setting your proper values. Even better if you use <input type="number" because then it won't be a text field, rather number.
  • footer link: returns 404 ( needs to be theme URI )
  • prefix $x - noticed one global $x
  • remove wp_title() as you already are declaring add_theme_support( 'title-tag' )
  • remove commented code
  • use the proper filter rather than having to create an entire class for the menu icons. There are two that I can think of that fall in that area: nav_menu_css_class and walker_nav_menu_start_el each work a little bit differently so do make sure to read the code and how they work. :)

Recommend:

  • use filters: post_class('col-md-4 col-sm-4 grid grid_2_column grid_3_column anim')
  • really advice against: <div id="primary" class="content-areas <?php do_action('etrigan_primary-width') ?>">

This has the potential to break the design if not used properly.

I'm not entirely sure how I feel about the way you are implementing the slider. This does have to be removed unfortunately. :(

It would be best if you found a way to use an existing plugin and make it work with that. Yes. I get not everybody will use the same slider plugin but you can adapt it to that. The key thing here is getting the content and presenting it if there is any. Otherwise, don't show it.

Curious as to why you are building your own searchform and adding support for core HTML5 search form? If you want to have your own you can use a filter and remove theme support for that otherwise you can use core markup and style that.

I'll reassign this back to you @holger1411 and if you are able to look over that, awesome, if not please let us know. @rohitink It's been great seeing your progress and themes while I've been reviewing themes for as long as I have. I do learn from your code as well. :)

#18 @jcastaneda
20 months ago

  • Status changed from reopened to reviewing

#19 @themetracbot
20 months ago

  • Summary changed from THEME: Etrigan – 0.91 to THEME: Etrigan – 0.93

Etrigan - 0.93

Etrigan is a very Powerful MultiPurpose theme Built with Bootstrap Framework. It supports WooCommerce and comes integrated with the latest Font Awesome Icons. There are multiple ways to showcase your Featured Products and Posts. Sidebar is fully Customizable with a Beautiful Recent Post Widget. There are so many Features that you won&#8217;t wanna switch this theme and it also Comes with Multiple Color Options. Demo Here: http://demo.rohitink.com/etrigan/

Theme URL - https://rohitink.com/2015/12/15/etrigan-multipurpose-theme/
Author URL - http://rohitink.com

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=etrigan/0.92&new_path=etrigan/0.93

History:

Ticket Summary Status Resolution Owner
#29217 THEME: Etrigan – 0.91 closed live holger1411

(this ticket)

#33977 THEME: Etrigan – 1.0 closed live themetracbot
#34087 THEME: Etrigan – 1.0.1 closed live themetracbot
#34185 THEME: Etrigan – 1.0.1.1 closed live themetracbot
#34239 THEME: Etrigan – 1.0.1.2 closed live themetracbot
#34674 THEME: Etrigan – 1.0.1.3 closed live themetracbot
#37173 THEME: Etrigan – 1.0.1.5 closed live themetracbot
#38230 THEME: Etrigan – 1.0.1.6 closed live themetracbot
#38538 THEME: Etrigan – 1.0.2 closed live themetracbot
#39341 THEME: Etrigan – 1.0.3 closed live themetracbot
#42283 THEME: Etrigan – 1.0.4 closed live themetracbot
#46385 THEME: Etrigan – 1.0.5 closed live themetracbot
#46558 THEME: Etrigan – 1.0.6 closed live themetracbot


https://themes.svn.wordpress.org/etrigan/0.93/screenshot.png

#20 @rohitink
20 months ago

  • Summary changed from THEME: Etrigan – 0.93 to THEME: Etrigan – 0.91

Hi @jcastaneda,

I have Fixed Many required issues. Except the last four.

  • prefix $x - noticed one global $x

I am not using any global $x. I have used them inside a for loop as a local and static variable only.
If I am mistaken, could you please point me in the right direction on where this global $x is.

  • remove wp_title() as you already are declaring add_theme_support( 'title-tag' )

I am not using wp_title() as well. Could you please tell me where did you see it?

  • remove commented code

I have kept a few commented code in my theme, to make it easier to update the theme in future and debug as well.

  • use the proper filter rather than having to create an entire class for the menu icons. There are two that I can think of that fall in that area: nav_menu_css_class and walker_nav_menu_start_el each work a little bit differently so do make sure to read the code and how they work. :)

nav_menu_css_class is not suitable for me, as i have 2 use cases here. What I have to use is walker_nav_menu_start_el. And if you notice carefully, that's what I am using at the end of the class. And I feel its better to use a class here as it adds consitency to the code, and makes it easier to read and debug. I don't think, there are rules to allow against use of a class. If I am wrong, please let me know.


For the slider, when I uploaded the theme, the rules were different. Its been 6 months :P
But, however I do understand the way I implemented the slider falls under Pseudo CPT and is not allowed. but, however I have consulted a few admins and key reviewers and I think its allowed if I make the slider not look like a Pseudo CPT.
Ulrich's comment here: https://themes.trac.wordpress.org/ticket/28698#comment:29

So I have made the change in my theme, and restricted the no. of slides to a max of 3. So, that its in no way as Pseudo CPT.

But I will also start working on a Slider Plugin, to work with my themes, and replace all the Slider section in my current themes with that Plugin.

And I have kept the HTML5 Search form support, just in case users choose to remove the search form in header, and use their own somewhere else in the theme.

And I am really honoured @jcastaneda that you feel progress in themes, and learn from my code. That has made my day :) :)

#21 @jcastaneda
20 months ago

The global is in the customizer file:

	function etrigan_show_slide_sec( $control ) {
	        $option = $control->manager->get_setting('etrigan_main_slider_count');
	        global $x;
	        if ( $x < $option->value() ){
	        	$x++;
	        	return true;
	        }
		}

For the slider, when I uploaded the theme, the rules were different. Its been 6 months :P

I wouldn't say that. The rule hasn't changed since I've been reviewing, I think the interpretation of the rule is what can cause the confusion there too. :) But glad to see you are understanding of this.

But I will also start working on a Slider Plugin,

Just use one of the already available: https://wordpress.org/plugins/search.php?q=slider

It makes more sense to provide support for other things already built than having to create something and then maintaining it down the road. Even cooler is using active callbacks in the customizer so that option is only shown when a plugin is active.

to make it easier to update the theme in future and debug as well.

Would you leave that code in a production site? Would you feel comfortable if you saw a crash test dummy leg in a car? That's what that debugging code can feel like.

And I have kept the HTML5 Search form support, just in case users choose to remove the search form in header, and use their own somewhere else in the theme.

Sounds fine, but please make a note of that somewhere in the docs just in case. There are some devs that may want to use solely core markup. :)

I am not using wp_title() as well. Could you please tell me where did you see it?

It is being used on: https://themes.svn.wordpress.org/etrigan/0.93/inc/extras.php

Let me know if you have any other questions. :)

#22 @themetracbot
20 months ago

  • Summary changed from THEME: Etrigan – 0.91 to THEME: Etrigan – 0.94

Etrigan - 0.94

Etrigan is a very Powerful MultiPurpose theme Built with Bootstrap Framework. It supports WooCommerce and comes integrated with the latest Font Awesome Icons. There are multiple ways to showcase your Featured Products and Posts. Sidebar is fully Customizable with a Beautiful Recent Post Widget. There are so many Features that you won&#8217;t wanna switch this theme and it also Comes with Multiple Color Options. Demo Here: http://demo.rohitink.com/etrigan/

Theme URL - https://rohitink.com/2015/12/15/etrigan-multipurpose-theme/
Author URL - http://rohitink.com

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=etrigan/0.93&new_path=etrigan/0.94

History:

Ticket Summary Status Resolution Owner
#29217 THEME: Etrigan – 0.91 closed live holger1411

(this ticket)

#33977 THEME: Etrigan – 1.0 closed live themetracbot
#34087 THEME: Etrigan – 1.0.1 closed live themetracbot
#34185 THEME: Etrigan – 1.0.1.1 closed live themetracbot
#34239 THEME: Etrigan – 1.0.1.2 closed live themetracbot
#34674 THEME: Etrigan – 1.0.1.3 closed live themetracbot
#37173 THEME: Etrigan – 1.0.1.5 closed live themetracbot
#38230 THEME: Etrigan – 1.0.1.6 closed live themetracbot
#38538 THEME: Etrigan – 1.0.2 closed live themetracbot
#39341 THEME: Etrigan – 1.0.3 closed live themetracbot
#42283 THEME: Etrigan – 1.0.4 closed live themetracbot
#46385 THEME: Etrigan – 1.0.5 closed live themetracbot
#46558 THEME: Etrigan – 1.0.6 closed live themetracbot


https://themes.svn.wordpress.org/etrigan/0.94/screenshot.png

#23 @rohitink
20 months ago

  • Summary changed from THEME: Etrigan – 0.94 to THEME: Etrigan – 0.91

@jcastaneda , Thanks so much for your input.
I understand and appreciate it all. I have all the required issues fixed now.
Let me know if there are more changes I need to make :)

#24 @holger1411
20 months ago

  • Status changed from reviewing to approved

#25 @jcastaneda
19 months ago

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

Awesome! Thanks for making those changes. Looks good.

Note: See TracTickets for help on using tickets.