WordPress.org

Make WordPress Themes

Opened 5 months ago

Closed 2 months ago

#45340 closed theme (not-approved)

THEME: Tyche – 1.0.11

Reported by: silkalns Owned by: acosmin
Priority: previously reviewed Keywords: theme-tyche
Cc: aigars.silkalns@…

Description

Tyche - 1.0.7

A WooCommerce theme

Theme URL - https://colorlib.com/themes/tyche
Author URL - https://colorlib.com/

Trac Browser - https://themes.trac.wordpress.org/browser/tyche/1.0.7

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=tyche/1.0.6&new_path=tyche/1.0.7

History:

Ticket Summary Status Resolution Owner
#43404 THEME: Tyche – 1.0.6 closed not-approved nabil_kadimi
#45340 THEME: Tyche – 1.0.11 closed not-approved acosmin

(this ticket)

#47201 THEME: Tyche – 1.0.13 closed live acosmin


https://themes.svn.wordpress.org/tyche/1.0.7/screenshot.png
Theme Check Results:

  • RECOMMENDED: No reference to add_theme_support( "custom-background", $args ) was found in the theme. If the theme uses background images or solid colors for the background, then it is recommended that the theme implement this functionality.
  • RECOMMENDED: No reference to add_editor_style() was found in the theme. It is recommended that the theme implement editor styling, so as to make the editor content match the resulting post output in the theme, for a better user experience.
  • RECOMMENDED: Could not find the file readme.txt in the theme. Please see Theme_Documentation for more information.
  • Warning: More than one text-domain is being used in this theme. This means the theme will not be compatible with WordPress.org language packs. The domains found are tyche, epsilon-framework

Change History (51)

#1 @poena
5 months ago

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

#2 @nabil_kadimi
5 months ago

HI @silkalns, Hi @machothemes let's continnue here:

So where were we? My Trello board says:

  1. [post-activation] Changing Static Front Page > Front Page to any value show a page without main content... Reverting to an empty choice shows content again.
  2. Undefined index error in: Appearance > About Tyche > Recommended Actions image
  3. Linking to non-existent documentation
  4. Recommended plugins not implemented in the theme image

Which of these have been fixed?

Last edited 5 months ago by nabil_kadimi (previous) (diff)

#3 @machothemes
5 months ago

@nabil_kadimi,

everything should be fixed in version 1.0.7. At least, they're not showing at my end anymore. Could you please, also check and let me know?

Thanks,
Cristian.

#4 @nabil_kadimi
4 months ago

For issue (1), I still can't set a static page, this is what I get when trying to set "Sample Page" as the front page Image

#5 @nabil_kadimi
4 months ago

Issue (3) still persists in About Tyche.

#6 @machothemes
4 months ago

HI @nabil_kadimi,

for issue(1), that's how it looks when you're actually setting a front-page. Why would you think it's broken?
for issue(3), documentation will be added after the theme's live. I don't see it as an "issue" since it's just a link that's currently leading to a 404 page. It's not very obvious or not presented as an important step.

If any admins consider this is an issue, we will fix it. Otherwise, I don't see a problem with this @nabil_kadimi

cc @poena

#7 @nabil_kadimi
4 months ago

Hi @machothemes

Issue (1), I expected that the cusomizer would show the page I chose.

Issue (3), I asked in Slack, links should point to the theme docs.

Last edited 4 months ago by nabil_kadimi (previous) (diff)

#8 @machothemes
4 months ago

@nabil_kadimi - just so we're clear and this is (hopefully) the last issue.

(1) - ok, we'll fix this but we'll need a time extension as our developer, that handled this theme, is on holiday; should be back in 6 days. Just letting you know so ticket doesn't get closed.
(3) - no longer an issue, correct?

Thanks,
Cristian.

#9 @nabil_kadimi
4 months ago

@machothemes

(1) Sure, just letting you know that I can't request reviewing other themes while reviewing yours.

(3) Who said that?

#10 @machothemes
4 months ago

@nabil_kadimi - update will be up soon, just letting you know.

#11 @themetracbot
4 months ago

  • Summary changed from THEME: Tyche – 1.0.7 to THEME: Tyche – 1.0.8

Tyche - 1.0.8

A WooCommerce theme

Theme URL - https://colorlib.com/themes/tyche
Author URL - https://colorlib.com/

Trac Browser - https://themes.trac.wordpress.org/browser/tyche/1.0.8

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=tyche/1.0.7&new_path=tyche/1.0.8

History:

Ticket Summary Status Resolution Owner
#43404 THEME: Tyche – 1.0.6 closed not-approved nabil_kadimi
#45340 THEME: Tyche – 1.0.11 closed not-approved acosmin

(this ticket)

#47201 THEME: Tyche – 1.0.13 closed live acosmin


https://themes.svn.wordpress.org/tyche/1.0.8/screenshot.png
Theme Check Results:

  • RECOMMENDED: No reference to add_theme_support( "custom-background", $args ) was found in the theme. If the theme uses background images or solid colors for the background, then it is recommended that the theme implement this functionality.
  • RECOMMENDED: No reference to add_editor_style() was found in the theme. It is recommended that the theme implement editor styling, so as to make the editor content match the resulting post output in the theme, for a better user experience.
  • RECOMMENDED: Could not find the file readme.txt in the theme. Please see Theme_Documentation for more information.
  • Warning: More than one text-domain is being used in this theme. This means the theme will not be compatible with WordPress.org language packs. The domains found are tyche, epsilon-framework

#12 @machothemes
4 months ago

Hi @nabil_kadimi

please check now. It should be ok.

Thanks,
Cristian.

#13 @nabil_kadimi
4 months ago

Hi @machothemes,

setup_postdata() is not allowed, it breaks the rule "Overriding WordPress globals is prohibited".

#14 @nabil_kadimi
4 months ago

So the new lisst is:

  1. Linking to inexistent documentation
  2. setup_postdata() is not allowed, it breaks the rule "Overriding WordPress globals is prohibited".

#15 @machothemes
4 months ago

Hello @nabil_kadimi,

setup_postdata function is being used by WooCommerce in their templates, and although i'm overriding ONLY the design, I have to copy/paste that file with all of its content in the theme.

Also, please take a look at line 84 where the global variable is being restored.

This ticket was mentioned in Slack in #themereview by nabil_kadimi. View the logs.


4 months ago

#17 @nabil_kadimi
4 months ago

Hello @machothemes

I asked on Slack and @poena said that's OK (link).

So the remaining issue is:

  1. Linking to inexistent documentation.

#18 @themetracbot
4 months ago

  • Summary changed from THEME: Tyche – 1.0.8 to THEME: Tyche – 1.0.9

Tyche - 1.0.9

A WooCommerce theme

Theme URL - https://colorlib.com/wp/themes/tyche/
Author URL - https://colorlib.com/

Trac Browser - https://themes.trac.wordpress.org/browser/tyche/1.0.9

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=tyche/1.0.8&new_path=tyche/1.0.9

History:

Ticket Summary Status Resolution Owner
#43404 THEME: Tyche – 1.0.6 closed not-approved nabil_kadimi
#45340 THEME: Tyche – 1.0.11 closed not-approved acosmin

(this ticket)

#47201 THEME: Tyche – 1.0.13 closed live acosmin


https://themes.svn.wordpress.org/tyche/1.0.9/screenshot.png
Theme Check Results:

  • RECOMMENDED: No reference to add_theme_support( "custom-background", $args ) was found in the theme. If the theme uses background images or solid colors for the background, then it is recommended that the theme implement this functionality.
  • RECOMMENDED: No reference to add_editor_style() was found in the theme. It is recommended that the theme implement editor styling, so as to make the editor content match the resulting post output in the theme, for a better user experience.
  • Warning: More than one text-domain is being used in this theme. This means the theme will not be compatible with WordPress.org language packs. The domains found are tyche, epsilon-framework

#19 @djrmom
4 months ago

@nabil_kadimi, are you still able to review this theme? If not, it can be reassigned. Thanks.

#20 @nabil_kadimi
4 months ago

@djrmom Yes sure, why would you want to reassign it?!

I'm still waiting for @machothemes response to my latest comment.

#21 @djrmom
4 months ago

@nabil_kadimi, a new version of theme has been uploaded since your previous response which you have not given a review on.

#22 @nabil_kadimi
4 months ago

@djrmom That version doesn't fix the remaining issue.

#23 @machothemes
4 months ago

@nabil_kadimi - what is the remaining issue ? Have you checked version 1.09 ?

#24 @machothemes
4 months ago

@nabil_kadimi - can I please get a reply to this?

#25 @nabil_kadimi
3 months ago

@nabil_kadimi - what is the remaining issue ?

Linking to inexistent documentation.

Have you checked version 1.09 ?

Yes

#26 @machothemes
3 months ago

@nabil_kadimi - Please, check again. Documentation has been already uploaded for this theme - you can see it here: https://colorlib.com/wp/support/tyche/ . If you are referring to a particular link/place in the theme - please let me know, specifically. Code line would be very beneficial to speed this up.

Thank you,
Cristian.

Last edited 3 months ago by machothemes (previous) (diff)

#28 @themetracbot
3 months ago

  • Summary changed from THEME: Tyche – 1.0.9 to THEME: Tyche – 1.0.10

Tyche - 1.0.10

A WooCommerce theme

Theme URL - https://colorlib.com/wp/themes/tyche/
Author URL - https://colorlib.com/

Trac Browser - https://themes.trac.wordpress.org/browser/tyche/1.0.10

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=tyche/1.0.9&new_path=tyche/1.0.10

History:

Ticket Summary Status Resolution Owner
#43404 THEME: Tyche – 1.0.6 closed not-approved nabil_kadimi
#45340 THEME: Tyche – 1.0.11 closed not-approved acosmin

(this ticket)

#47201 THEME: Tyche – 1.0.13 closed live acosmin


https://themes.svn.wordpress.org/tyche/1.0.10/screenshot.png
Theme Check Results:

  • RECOMMENDED: No reference to add_theme_support( "custom-background", $args ) was found in the theme. If the theme uses background images or solid colors for the background, then it is recommended that the theme implement this functionality.
  • RECOMMENDED: No reference to add_editor_style() was found in the theme. It is recommended that the theme implement editor styling, so as to make the editor content match the resulting post output in the theme, for a better user experience.
  • RECOMMENDED: Tags: is either empty or missing in style.css header.
  • Warning: More than one text-domain is being used in this theme. This means the theme will not be compatible with WordPress.org language packs. The domains found are tyche, epsilon-framework

#29 @themetracbot
3 months ago

  • Summary changed from THEME: Tyche – 1.0.10 to THEME: Tyche – 1.0.11

Tyche - 1.0.11

A WooCommerce theme

Theme URL - https://colorlib.com/wp/themes/tyche/
Author URL - https://colorlib.com/

Trac Browser - https://themes.trac.wordpress.org/browser/tyche/1.0.11

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=tyche/1.0.10&new_path=tyche/1.0.11

History:

Ticket Summary Status Resolution Owner
#43404 THEME: Tyche – 1.0.6 closed not-approved nabil_kadimi
#45340 THEME: Tyche – 1.0.11 closed not-approved acosmin

(this ticket)

#47201 THEME: Tyche – 1.0.13 closed live acosmin


https://themes.svn.wordpress.org/tyche/1.0.11/screenshot.png
Theme Check Results:

  • RECOMMENDED: No reference to add_theme_support( "custom-background", $args ) was found in the theme. If the theme uses background images or solid colors for the background, then it is recommended that the theme implement this functionality.
  • RECOMMENDED: No reference to add_editor_style() was found in the theme. It is recommended that the theme implement editor styling, so as to make the editor content match the resulting post output in the theme, for a better user experience.
  • Warning: More than one text-domain is being used in this theme. This means the theme will not be compatible with WordPress.org language packs. The domains found are tyche, epsilon-framework

#30 @nabil_kadimi
3 months ago

  • Status changed from reviewing to approved

This looks good to me.

There are a few minor issues I noticed, but this won't prevent me from approving the theme:

#31 @kevinhaig
2 months ago

I will do this final.

#32 @kevinhaig
2 months ago

  • Status changed from approved to reopened

Theme Review - Tyche Version 1.0.7

  • Hi I am @kevinhaig and I have completed the second (final) review of your theme.
  • Sorry but the ticket has been reopened because there are requirements not met.
  • The review process follows procedures found in the theme handbook.
  • ref: https://make.wordpress.org/themes/handbook/review/
  • If you do not understand or agree with something in the review, please comment in the ticket.
  • I can then help you, or if I am not sure of something I will certainly seek a second opinion from another key reviewer.

Responding to the Review

  • Please respond with update or request an extension within 7 days, or theme may be closed.

Required Items

Code

All untrusted data should be escaped before output.
  • get_permalink must be escaped, use esc_url() , file: tyche\inc\helpers\class-tyche-helper.php, line: 311
  • get_permalink must be escaped, use esc_url() , file: tyche\inc\helpers\class-tyche-helper.php, line: 314
  • get_permalink must be escaped, use esc_url() , file: tyche\inc\libraries\class-tyche-breadcrumbs.php, line: 310
  • get_permalink must be escaped, use esc_url() , file: tyche\inc\libraries\class-tyche-breadcrumbs.php, line: 428
  • Expected next thing to be an escaping function (see Codex for 'Data Validation'), not 'get_previous_posts_link' , file: tyche\inc\libraries\class-tyche-woocommerce-hooks.php, line: 180
  • Expected next thing to be an escaping function (see Codex for 'Data Validation'), not 'get_next_posts_link' , file: tyche\inc\libraries\class-tyche-woocommerce-hooks.php, line: 221
  • Expected next thing to be an escaping function (see Codex for 'Data Validation'), not '$this' , file: tyche\inc\libraries\epsilon-framework\controls\class-epsilon-control-color-scheme.php, line: 37
  • Expected next thing to be an escaping function (see Codex for 'Data Validation'), not '$this' , file: tyche\inc\libraries\epsilon-framework\controls\class-epsilon-control-color-scheme.php, line: 41
  • Expected next thing to be an escaping function (see Codex for 'Data Validation'), not '$choice' , file: tyche\inc\libraries\epsilon-framework\controls\class-epsilon-control-color-scheme.php, line: 44
  • Expected next thing to be an escaping function (see Codex for 'Data Validation'), not '$choice' , file: tyche\inc\libraries\epsilon-framework\controls\class-epsilon-control-color-scheme.php, line: 46
  • Expected next thing to be an escaping function (see Codex for 'Data Validation'), not '$color' , file: tyche\inc\libraries\epsilon-framework\controls\class-epsilon-control-color-scheme.php, line: 49
  • Expected next thing to be an escaping function (see Codex for 'Data Validation'), not '$this' , file: tyche\inc\libraries\epsilon-framework\controls\class-epsilon-control-slider.php, line: 55
  • Expected next thing to be an escaping function (see Codex for 'Data Validation'), not '$this' , file: tyche\inc\libraries\epsilon-framework\controls\class-epsilon-control-slider.php, line: 60
  • Expected next thing to be an escaping function (see Codex for 'Data Validation'), not '$this' , file: tyche\inc\libraries\epsilon-framework\controls\class-epsilon-control-slider.php, line: 63
  • Expected next thing to be an escaping function (see Codex for 'Data Validation'), not '$this' , file: tyche\inc\libraries\epsilon-framework\controls\class-epsilon-control-slider.php, line: 66
  • Expected next thing to be an escaping function (see Codex for 'Data Validation'), not '$this' , file: tyche\inc\libraries\epsilon-framework\controls\class-epsilon-control-slider.php, line: 67
  • Expected next thing to be an escaping function (see Codex for 'Data Validation'), not '$this' , file: tyche\inc\libraries\epsilon-framework\controls\class-epsilon-control-slider.php, line: 68
  • Expected next thing to be an escaping function (see Codex for 'Data Validation'), not '$this' , file: tyche\inc\libraries\epsilon-framework\controls\class-epsilon-control-slider.php, line: 70
  • Expected next thing to be an escaping function (see Codex for 'Data Validation'), not '$this' , file: tyche\inc\libraries\epsilon-framework\controls\class-epsilon-control-slider.php, line: 73
  • Expected next thing to be an escaping function (see Codex for 'Data Validation'), not '$this' , file: tyche\inc\libraries\epsilon-framework\controls\class-epsilon-control-slider.php, line: 73
  • Expected next thing to be an escaping function (see Codex for 'Data Validation'), not '$this' , file: tyche\inc\libraries\epsilon-framework\controls\class-epsilon-control-slider.php, line: 74
  • Expected next thing to be an escaping function (see Codex for 'Data Validation'), not '$this' , file: tyche\inc\libraries\epsilon-framework\controls\class-epsilon-control-slider.php, line: 75
  • Expected next thing to be an escaping function (see Codex for 'Data Validation'), not '$size' , file: tyche\sidebar-footer.php, line: 82
  • Do not use esc_html() or esc_html_e() to escape an attribute link. Use esc_url( ( 'my_link', 'my-text-domain' ) ) because it is a link. , file: tyche\template-parts\main-slider.php, line: 47
  • Do not use esc_html() or esc_html_e() to escape an attribute link. Use esc_url( ( 'my_link', 'my-text-domain' ) ) because it is a link. , file: tyche\template-parts\main-slider.php, line: 50
  • Expected next thing to be an escaping function (see Codex for 'Data Validation'), not '$current_flag' , file: tyche\template-parts\top-header.php, line: 46
  • get_permalink must be escaped, use esc_url() , file: tyche\woocommerce\single-product\add-to-cart\grouped.php, line: 70

Core Functionality and Features

Use WordPress functionality and features first, if available.
  • add_theme_support() for custom-logo used but not found in tags list in style.css , file: tyche\inc\class-tyche.php, line: 282

Other Required Items

Other Required Items
  • You are not allowed to load 'wp-load.php' from within a theme. , file: tyche\inc\libraries\welcome-screen\class-epsilon-welcome-screen.php, line: 350
  • You are not allowed to load 'wp-admin/admin.php' from within a theme. , file: tyche\inc\libraries\welcome-screen\class-epsilon-welcome-screen.php, line: 351, ref: https://wordpress.slack.com/archives/C02RP4Y3K/p1470952005002504
  • You are not allowed to load 'wp-admin/admin-header.php' from within a theme. , file: tyche\inc\libraries\welcome-screen\class-epsilon-welcome-screen.php, line: 352
  • Overriding WordPress globals is prohibited , file: tyche\woocommerce\single-product\add-to-cart\grouped.php, line: 37 ...Not sure why this is coming up, may need a second opinion.
  • Overriding WordPress globals is prohibited , file: tyche\woocommerce\single-product\add-to-cart\grouped.php, line: 84...Not sure why this is coming up, may need a second opinion.
  • don't declare images or fonts if they are not bundled with the theme.
  • plugin installer does not appear to be working
No Theme Copyright in Footer
  • Do not put your copyright in the footer. It implies that all websites that use your theme are copyright by you and I don't think you want that. Typically theme authors grab the site title for this. Even if it is a default, if the user does not change the default you do not want to suggest that the website is copyright by you!
    • the way you have set up the default reads like you are copyright 'ing the site to theme Tyche. Put the site copyright declaration on the left hand side.

#33 follow-up: @greenshady
2 months ago

The overriding WP globals thing is a false-positive. Well, it shouldn't be noted in this case. And, I've explained this before to the folks building the theme sniffer.

Essentially, yes, the theme is overriding the global $post variable. However, this is one the legit reasons to do it. Doing this along with get_posts() and setup_postdata() is the only way to actually get the standard WP template tags working. You have to let WP know about the current post object in loop.

#34 in reply to: ↑ 33 @kevinhaig
2 months ago

Replying to greenshady:

The overriding WP globals thing is a false-positive. Well, it shouldn't be noted in this case. And, I've explained this before to the folks building the theme sniffer.

Essentially, yes, the theme is overriding the global $post variable. However, this is one the legit reasons to do it. Doing this along with get_posts() and setup_postdata() is the only way to actually get the standard WP template tags working. You have to let WP know about the current post object in loop.

Thanks, I thought it should not be coming up.

Last edited 2 months ago by kevinhaig (previous) (diff)

#35 follow-up: @machothemes
2 months ago

@kevinhaig - will take a look at this tomorrow morning (Monday).

Thanks for your detailed review. I'm not sure though about what the sniffer's reporting in re. to this:
Expected next thing to be an escaping function (see Codex for 'Data Validation'), not '$this' , file:

Have you also checked manually or just posted the output of the sniffer here? Want to understand better if this is something you're also considering an issue or something that hasn't yet been checked by a person and just a report by sniffer.

Thanks,
Cristian.

#36 in reply to: ↑ 35 @kevinhaig
2 months ago

Replying to machothemes:

@kevinhaig - will take a look at this tomorrow morning (Monday).

Thanks for your detailed review. I'm not sure though about what the sniffer's reporting in re. to this:
Expected next thing to be an escaping function (see Codex for 'Data Validation'), not '$this' , file:

Have you also checked manually or just posted the output of the sniffer here? Want to understand better if this is something you're also considering an issue or something that hasn't yet been checked by a person and just a report by sniffer.

Thanks,
Cristian.

The sniffer reported 3 times as many errors. I went through each one, and have just listed the ones I think need to be looked at. In some cases I was not sure where the data was coming from so I included the error.

At the end of the day it is your theme out there. So if in doubt about escaping, escape it. There is no question that some of the $this->#variables need escaping, I'm just not sure about all of them.

#37 @machothemes
2 months ago

@kevinhaig - I'm not sure the copyright "issue" is actually an issue. Seems to be more like a personal choice and not required. Can you please confirm this?

#38 @kevinhaig
2 months ago

A little searching on slack will find you your answer.

https://wordpress.slack.com/archives/C02RP4Y3K/p1490296742259564

#39 @kevinhaig
2 months ago

It's also in the requirements under licensing

Any copyright statements on the front end should display the user’s copyright, not the theme author’s copyright.

#40 @machothemes
2 months ago

@kevinhaig I'm talking about this :

Put the site copyright declaration on the left hand side.

Copyright placement is a design choice and I don't believe it's required to be in a certain alignment.

#41 @kevinhaig
2 months ago

Look ... the following
Copyright (C) 2017 | Theme:Tyche | Powered by WordPress

reads like the theme author owns the site, that is not allowed

if you did the following:

Copyright (C) 2017 User Site Title | Theme:Tyche | Powered by WordPress

then that would be fine as well.

I don't care how you do it but do not leave the impression that the copyright is the theme author copyright.

#42 @greenshady
2 months ago

This looks OK to me and doesn't break the copyright text rule. The reason is because of the clear separators between the three items in the footer text. With the separator there, it's clear that the copyright statement is not tied to the other two parts.

Not acceptable:

Copyright (C) 2017 Tyche | Powered by WordPress

Acceptable but could be improved:

Copyright (C) 2017 | Theme:Tyche | Powered by WordPress

Awesomesauce:

Copyright (C) 2017 Site Title | Theme:Tyche | Powered by WordPress

Translating

Also, just a note that we should be internationalizing the text. This:

<?php echo wp_kses_post( get_theme_mod( 'tyche_copyright_contents', sprintf( 'Copyright &copy; ' . date( 'Y' ) . ' <span class="sep">|</span> <a href="%s">Theme: Tyche</a> <span class="sep">|</span> Powered by WordPress.', 'https://colorlib.com/tyche/' ) ) ); ?>

Should be something more like:

<?php echo wp_kses_post( 
	get_theme_mod( 
		'tyche_copyright_contents', 
		sprintf( 
			// Translators: 1 is current year, 2 is separator, 3 is theme link.
			__( 'Copyright &copy; %1$s %2$s %3$s %$2s Powered by WordPress.', 'tyche' ),
			date_i18n( 'Y' ),
			'<span class="sep">|</span>',
			sprintf( '<a href="https://colorlib.com/tyche">%s</a>', __( 'Theme: Tyche', 'tyche' ) ),
		)
	)
); ?>

#43 @kevinhaig
2 months ago

Sorry @greenshady, it reads to me like a separator of the copyright declaration from the theme name from the WordPress credit.

The author intends absolutely to leave that impression, and that is the impression it has left me.

If it was not a big deal to the author then add the site title......simple. Na .... @machothemes knows what he is inferring.

#44 @kevinhaig
2 months ago

Hey @greenshady , will you please pick up the final, I really don't want to review this theme any more :)

#45 @machothemes
2 months ago

@kevinhaig - I'm really sorry you feel that way. It was never our intention to make it look like the copyright belong to us. Until you signalled it I honestly wasn't even able to see it that way.

There's no need to get alarmed as we have absolutely no intention of inferring anything bad. We're waiting for a patch to be approved on theme's GitHub page (see: https://github.com/puikinsh/tyche/pull/35), with the issue Justin signalled above in re. to how we handle the footer text translation and will upload a version of the theme with the footer copyright issue (the one you signalled) addressed.

As you can also see from this commit: https://github.com/puikinsh/tyche/pull/34/commits/b865e8499069c5efda7d3c2b7cc5a18db50f4d36 (happened ~ 6 hours ago), we had already addressed this issue. Due to the language barrier, we understood we have to move the copyright message to the left side of the footer - instead of the right one, where it's currently positioned.

Our message here: https://themes.trac.wordpress.org/ticket/45340#comment:40 was questioning the fact that you were asking us to place the copyright message on the left side, the argument that the messages' positioning is a design choice still stands IMHO.

FWIW we're very open to learning new things and are open to discussion any time and we really appreciate the fact you took the time to review this theme.

However, I can understand if you don't want to finish the review - it's not something we wanted to happen in any way, but it's your decision completely and we'll respect it no matter what.

Thank you for taking the time to read this,
Cristian.

#46 @kevinhaig
2 months ago

  • Owner changed from nabil_kadimi to kevinhaig
  • Status changed from reopened to reviewing

#47 @kevinhaig
2 months ago

  • Status changed from reviewing to approved

Next reviewer.

Note that this theme still needs follow up on the above initial final review.

This ticket was mentioned in Slack in #themereview by cristianraiber. View the logs.


2 months ago

#49 @acosmin
2 months ago

  • Status changed from approved to reopened

Continuing review in this ticket: https://themes.trac.wordpress.org/ticket/47201

#50 @acosmin
2 months ago

  • Owner changed from kevinhaig to acosmin
  • Status changed from reopened to reviewing

#51 @acosmin
2 months ago

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