WordPress.org

Make WordPress Themes

Opened 3 months ago

Closed 2 weeks ago

Last modified 2 weeks ago

#44682 closed theme (closed-newer-version-uploaded)

THEME: Reykjavik – 0.9.4

Reported by: webmandesign Owned by: alkesh7
Priority: new theme Keywords: theme-reykjavik accessibility-ready
Cc: webmandesigneu@…

Description

Reykjavik - 0.9.0

Reykjavik is fresh, lightweight, speed and SEO optimized, accessibility ready WordPress theme perfect for your next business, portfolio, blog or WooCommerce e-shop website. You can customize all elements of the theme to your needs. The theme works perfectly with Beaver Builder, Beaver Themer, Elementor, Visual Composer and other page builders to create fantastic layouts. It features mobile-optimized codebase and design with unique, easy-to-access mobile navigation. Build your website in no time with integrated one-click demo import functionality. Let’s impress your website visitors with this beautiful theme now!

Theme URL - https://www.webmandesign.eu/portfolio/reykjavik-wordpress-theme/
Author URL - https://www.webmandesign.eu/

Trac Browser - https://themes.trac.wordpress.org/browser/reykjavik/0.9.0

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

History:

Ticket Summary Status Resolution Owner
#44682 THEME: Reykjavik – 0.9.4 closed closed-newer-version-uploaded alkesh7

(this ticket)

#46546 THEME: Reykjavik – 0.9.11 reviewing


https://themes.svn.wordpress.org/reykjavik/0.9.0/screenshot.jpg

Change History (32)

#1 @themetracbot
2 months ago

  • Keywords changed from theme-reykjavik, accessibility-ready to theme-reykjavik accessibility-ready
  • Summary changed from THEME: Reykjavik – 0.9.0 to THEME: Reykjavik – 0.9.1

Reykjavik - 0.9.1

Reykjavik is fresh, lightweight, speed and SEO optimized, accessibility ready WordPress theme perfect for your next business, portfolio, blog or WooCommerce e-shop website. You can customize all elements of the theme to your needs. The theme works perfectly with Beaver Builder, Beaver Themer, Elementor, Visual Composer and other page builders to create fantastic layouts. It features mobile-optimized codebase and design with unique, easy-to-access mobile navigation. Build your website in no time with integrated one-click demo import functionality. Impress your website visitors with this beautiful theme now!

Theme URL - https://www.webmandesign.eu/portfolio/reykjavik-wordpress-theme/
Author URL - https://www.webmandesign.eu/

Trac Browser - https://themes.trac.wordpress.org/browser/reykjavik/0.9.1

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=reykjavik/0.9.0&new_path=reykjavik/0.9.1

History:

Ticket Summary Status Resolution Owner
#44682 THEME: Reykjavik – 0.9.4 closed closed-newer-version-uploaded alkesh7

(this ticket)

#46546 THEME: Reykjavik – 0.9.11 reviewing


https://themes.svn.wordpress.org/reykjavik/0.9.1/screenshot.jpg

#2 @joyously
8 weeks ago

Here is some user feedback to consider before your review.

  • On activation, the theme outputs this message: "The main theme CSS stylesheet was regenerated. Please refresh your web browser's and server's cache (if you are using a website server caching solution)." This is obnoxious and unnecessary. If you use a version number on your style sheet, you will not need to tell the admin anything about this, and the site visitors will get the correct version also. Besides, the theme is not supposed to write defaults anywhere, so nothing should have happened on activation.
  • Add descriptions to the widget areas to indicate when and where they are shown.
  • Theme or author name/brand should not be output on front end except as allowed in a credit link. Remove the theme logo that is shown by default. No one will want the theme logo.
  • The menu needs to have a fallback for when no menu is selected. The default value of fallback_cb parameter of wp_nav_menu() works really well, and it's core functionality.
  • I do not like the extra text that is in the menu. (category descriptions)
  • Any restrictions (such as menu depth=4) need to be documented for the user.
  • The mobile menu X ix partially covered by the admin bar.
  • Themes may have a single credit link in the footer. Theme URI or Author URI
  • A post title with markup renders correctly in the blog page, but not in the single post page.
  • img[width], img[height] {height: auto;} This should be on every image, not just the ones with HTML attributes. It goes with max-width:100% so that the images do not look distorted.
  • Links in image captions look like the rest of the text.
  • The comment input box just looks like a line. I can't tell that it is a place to type. Same for search widget.
  • Remove the CSS counters for user content ordered lists. It is not taking into account the start attribute or the reversed attribute. Here is the same page twice: on the left how the theme is displaying, but on the right how they are without the counters (I used Firebug to disable them). https://s2.postimg.org/gau3xe87t/ordered-list-numbers.jpg
  • You might want to hide the menu, comment form, and widget areas for the print styles. Also, the comment author avatars are showing really large.
  • Pingbacks and trackbacks should be shown.
  • Gallery captions are only shown on hover. This obscures the image, and how are touch screen users supposed to see them? Also, a long caption is truncated.
  • .gallery img {width: 100%;} This CSS rule should be removed, as the theme cannot know what the image width should be. It causes the browser to upscale thumbnails, which looks awful, and the user doesn't get the size he chose.
  • Same for .entry-media img {width: 100%;}
  • The image attachment page shows the image size, but does not display the image at that size.
  • The image attachment page has no post navigation links. (back to gallery or Previous/Next attachment)
  • Can you put the sidebar on the 404 page?
  • Customizer
    • Can you make all checkboxes consistent with core options and boolean logic, so that a checked option means "Enabled" or "Show" instead of "Disabled" or "Hide", please?
    • Header Media - the Header Image has no default, but the suggested image is shown on archive pages. I don't see a way to remove it.

#3 @joyously
7 weeks ago

  • When I looked at the next theme, I noticed that my Media Settings had been changed by this theme. Themes are not allowed to update any options but theme options. Remove the code to change the image sizes. Themes can add image sizes to suit the design, but cannot change the standard image sizes.

#4 @themetracbot
7 weeks ago

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

#5 @alkesh7
7 weeks ago

Hi @webmandesign,

I am reviewing your theme.

Please complete all the changes what our theme reviewer team suggested on your theme and upload new copy of theme as next version theme with the all changes as completed/resolved.

Thanks!

#6 @themetracbot
7 weeks ago

  • Summary changed from THEME: Reykjavik – 0.9.1 to THEME: Reykjavik – 0.9.2

Reykjavik - 0.9.2

Reykjavik is fresh, lightweight, speed and SEO optimized, accessibility ready WordPress theme perfect for your next business, portfolio, blog or WooCommerce e-shop website. You can customize all elements of the theme to your needs. The theme works perfectly with Beaver Builder, Beaver Themer, Elementor, Visual Composer and other page builders to create fantastic layouts. It features mobile-optimized codebase and design with unique, easy-to-access mobile navigation. Build your website in no time with integrated one-click demo import functionality. Impress your website visitors with this beautiful theme now!

Theme URL - https://www.webmandesign.eu/portfolio/reykjavik-wordpress-theme/
Author URL - https://www.webmandesign.eu/

Trac Browser - https://themes.trac.wordpress.org/browser/reykjavik/0.9.2

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=reykjavik/0.9.1&new_path=reykjavik/0.9.2

History:

Ticket Summary Status Resolution Owner
#44682 THEME: Reykjavik – 0.9.4 closed closed-newer-version-uploaded alkesh7

(this ticket)

#46546 THEME: Reykjavik – 0.9.11 reviewing


https://themes.svn.wordpress.org/reykjavik/0.9.2/screenshot.jpg

#7 @webmandesign
7 weeks ago

Hi @joyously and @alkesh7,

Thank you very much for your thorough and informative feedback, @joyously

Here are improvements I've done in theme version 0.9.2 and some explanations to your feedback:

  1. [DISCUSSION NEEDED] "The main theme CSS stylesheet was regenerated..." notice - The theme actually generates a single stylesheet that includes user custom theme styles set via customizer theme options in WordPress' uploads folder. I use a theme version for stylesheet enqueuing. However, this stylesheet is being regenerated after each customizer settings update. I decided to let users know of this by displaying the admin notice, so they can take appropriate actions if they experience caching issues. Not sure if this is really a bad practice, please let me know if it is and provide me with some them review requirements too, please. I always strive to build my themes according to WP theme review requirements.
  2. [FIXED] Widget areas descriptions - Done.
  3. [DISCUSSION NEEDED] Theme or author name/brand - The theme default logo is only set if false === get_theme_mod( 'custom_logo' ). This is to preview theme capabilities to user to showcase how logo image would look like in their theme. Once you set a custom logo or remove the custom logo altogether, the theme will not show the default logo image. So, is this still required to be removed from theme code? How about the theme screenshot then?
  4. [FIXED] Menu fallback - I know of the fallback_cb parameter and I'm actually using it where appropriate - in primary menu. I set this to a message explaining how to set up custom WordPress menus. As for other menus in the theme, this is not really required as the menus simply doesn't have to be displayed - it's really up to theme user to set them or not. And that's why I set 'fallback_cb' => false for those menus (secondary and social).
  5. [FIXED] Extra text in the menu - I'm not quite sure what you mean here. But I think you mean a taxonomy menu item description once you add a taxonomy with a description into your custom menu. Well, this is native WordPress behavior and I have no control over that, though I don't quite like it. But as you are building a custom menu, you can either remove the menu item description text or set your own custom one. Just enable the menu description field in Screen Options.
  6. [FIXED] Menu depth restrictions - I've added this information into theme documentation (you can actually find this in reykjavik/documentation folder and also online).
  7. [FIXED] Mobile menu X partially covered by admin bar - Thank you for spotting this, I've fixed it now.
  8. [FIXED] Themes may have a single credit link in the footer. Theme URI or Author URI - Again, thank you for pointing this out. How come I misunderstood that :)
  9. [FIXED] Markup in post title - Thanks for spotting this. Fixed now.
  10. [FIXED] Image height styles - Fixed. This style was actually some legacy from old IE issue...
  11. [FIXED] Links in image caption - The color was actually different, but almost unrecognizable. I've fixed this now.
  12. [FIXED] Form fields design - Well, this is a theme design decision. There are actually a form field placeholders to guide you where to type. I'm sorry, I will not change this as it is related to theme design and if I make a different style for textarea and for text input field, it would be inconsistent. I'll wait what accessibility review tells about this. But thank you for the opinion here!
  13. [FIXED] Custom ordered list styles - I fixed these. (Just an info note: I've actually used theme unit test from 2nd June to test the theme but I can see now that you have updated that...)
  14. [FIXED] Print styles - I'm sorry but I've decided not to provide print styles for this theme. Is it required for themes?
  15. [FIXED] Pingbacks and trackbacks - I've enabled them.
  16. [FIXED] Gallery captions - Fixed.
  17. [FIXED] Remove .gallery img {width: 100%;} - This is quite unfortunate, but I've fixed it.
  18. [FIXED] Remove .entry-media img {width: 100%;} - The same as 17. above.
  19. [FIXED] Image display on attachment page - This is intentional. I actually display an image thumbnail for faster page load and link this image to the original image size.
  20. [FIXED] Attachment page navigation - Fixed.
  21. [FIXED] Sidebar on 404 page - Sorry, this is intentional decision for theme design. I will not display the sidebar on 404 page.
  22. [FIXED] Customizer - I've fixed checkboxes logic, though, it made some code more complicated... But I'm not sure what you mean by header image on archive pages issue - the default in theme is set to randomize images and you can simply hide the image clicking the "Hide image" button in Customizer > Header Media. I don't experience any issue with that.
  23. [FIXED] [DISCUSSION NEEDED] Image sizes reset - Well, this is a topic for more discussion I think. Media sizes are tightly related to theme you use I think. And I've actually got a lot of users asking for reducing amount of image sizes created by my themes. That's why I've started to use WordPress default image sizes as WP creates 4 of them already (+ 1 fullsize). Adding more custom image sizes is really a pain for users on low cost servers. That's why I actually set the WordPress media sizes to theme recommended ones during the theme installation process. You can adapt those size to your wish later, even when using the theme, actually. I've noticed I've had the script that puts the sizes back to previous state disabled in the theme, so I've reenabled it again and everything should work fine now. Please, let me know if this is sufficient. (If I'm not mistaken, I actually use this method in my other WPORG themes too.)

Best regards,

Oliver

Last edited 7 weeks ago by webmandesign (previous) (diff)

#8 @joyously
7 weeks ago

  1. [DISCUSSION NEEDED] "The main theme CSS stylesheet was regenerated..." notice - The theme actually generates a single stylesheet that includes user custom theme styles set via customizer theme options in WordPress' uploads folder. I use a theme version for stylesheet enqueuing. However, this stylesheet is being regenerated after each customizer settings update. I decided to let users know of this by displaying the admin notice, so they can take appropriate actions if they experience caching issues. Not sure if this is really a bad practice, please let me know if it is and provide me with some them review requirements too, please. I always strive to build my themes according to WP theme review requirements.

There is still a problem, because I did not use the Customizer. All I did was activate the theme and that notice was displayed. Themes must not write defaults to the database or the file system. If you use version number on the stylesheet, there should be no caching problems.

  1. [DISCUSSION NEEDED] Theme or author name/brand - The theme default logo is only set if false === get_theme_mod( 'custom_logo' ). This is to preview theme capabilities to user to showcase how logo image would look like in their theme. Once you set a custom logo or remove the custom logo altogether, the theme will not show the default logo image. So, is this still required to be removed from theme code? How about the theme screenshot then?

There should not be a default logo; the site title should be the default. The default logo is showing on the front end. That should not ever happen. You can have it set in your screenshot, but not as a default in the theme.

  1. [FIXED] Menu fallback - I know of the fallback_cb parameter and I'm actually using it where appropriate - in primary menu. I set this to a message explaining how to set up custom WordPress menus. As for other menus in the theme, this is not really required as the menus simply doesn't have to be displayed - it's really up to theme user to set them or not. And that's why I set fallback_cb => false for those menus (secondary and social).

Yes, it is only the primary menu that I am concerned with for a fallback. By setting it to an installation message, the site visitor is left without a menu. (installation instructions should not be visible to site visitors) The core fallback default is an actual menu. Use core functionality first.

  1. [FIXED] Extra text in the menu - I'm not quite sure what you mean here. But I think you mean a taxonomy menu item description once you add a taxonomy with a description into your custom menu. Well, this is native WordPress behavior and I have no control over that, though I don't quite like it. But as you are building a custom menu, you can either remove the menu item description text or set your own custom one. Just enable the menu description field in Screen Options.

What exactly did you fix if you didn't understand? Yes, I was referring to the taxonomy menu item descriptions, but they were only showing in this theme, not other themes I have tested with the exact same menu. So there is control over that, and don't expect the user to change something just because the theme couldn't figure out the menu system.

  1. [FIXED] Customizer - But I'm not sure what you mean by header image on archive pages issue - the default in theme is set to randomize images and you can simply hide the image clicking the "Hide image" button in Customizer > Header Media. I don't experience any issue with that.

There is an image shown on Archive pages that looks like a header image. When I try to see how to remove it, it is only shown under "Suggested". There is no "Hide Image" button if there is no header image set, so there is no way to remove it. If it is the default, it should show as the default, so the user can remove it. If not the default, it should not show. Options must work as expected.

  1. [FIXED] [DISCUSSION NEEDED] Image sizes reset - Well, this is a topic for more discussion I think. Media sizes are tightly related to theme you use I think. And I've actually got a lot of users asking for reducing amount of image sizes created by my themes. That's why I've started to use WordPress default image sizes as WP creates 4 of them already (+ 1 fullsize). Adding more custom image sizes is really a pain for users on low cost servers. That's why I actually set the WordPress media sizes to theme recommended ones during the theme installation process. You can adapt those size to your wish later, even when using the theme, actually. I've noticed I've had the script that puts the sizes back to previous state disabled in the theme, so I've reenabled it again and everything should work fine now. Please, let me know if this is sufficient. (If I'm not mistaken, I actually use this method in my other WPORG themes too.)

There is nothing to discuss here. Themes may not change options outside of theme options. The theme can use the core names "large", "full", "thumbnail", "medium", but cannot set the sizes of those. Changing those sizes only affects future uploads, but does nothing to change the existing files. It should not matter to the theme what sizes the user chooses for his images, because the theme styles should work for all sizes, and the user should get the sizes that he chose.

#9 @themetracbot
7 weeks ago

  • Summary changed from THEME: Reykjavik – 0.9.2 to THEME: Reykjavik – 0.9.3

Reykjavik - 0.9.3

Reykjavik is fresh, lightweight, speed and SEO optimized, accessibility ready WordPress theme perfect for your next business, portfolio, blog or WooCommerce e-shop website. You can customize all elements of the theme to your needs. The theme works perfectly with Beaver Builder, Beaver Themer, Elementor, Visual Composer and other page builders to create fantastic layouts. It features mobile-optimized codebase and design with unique, easy-to-access mobile navigation. Build your website in no time with integrated one-click demo import functionality. Impress your website visitors with this beautiful theme now!

Theme URL - https://www.webmandesign.eu/portfolio/reykjavik-wordpress-theme/
Author URL - https://www.webmandesign.eu/

Trac Browser - https://themes.trac.wordpress.org/browser/reykjavik/0.9.3

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=reykjavik/0.9.2&new_path=reykjavik/0.9.3

History:

Ticket Summary Status Resolution Owner
#44682 THEME: Reykjavik – 0.9.4 closed closed-newer-version-uploaded alkesh7

(this ticket)

#46546 THEME: Reykjavik – 0.9.11 reviewing


https://themes.svn.wordpress.org/reykjavik/0.9.3/screenshot.jpg

#10 @webmandesign
7 weeks ago

Hi,

Thanks again for your input, @joyously

Here are changes I've made in theme version 0.9.3:

1) [FIXED] - Just to let you know, I was also generating stylesheet upon theme activation, that's why you got the admin notice. This logic was removed from the theme and it only refreshes the stylesheet after saving customizer. Also timestamp version was added to generated stylesheet.
3) [FIXED] - I don't display default theme logo any more.
4) [FIXED] - I use default menu fallback for Primary navigation.
23) [FIXED] - The functionality to set custom image sizes was removed in favor of only informing user of the recommended image sizes.


22) [FIXED], but - This is actually an issue within WordPress itself I think:

I only use default WordPress functions to register custom header. I've had it set to 'random-default' => true. This is useful when you provide multiple custom header images. The issue here is that by default it won't display "Hide image" button in WordPress customizer. But this is not an issue with my theme as you may understand.

The workaround is to set default-image parameter for custom headers. This will display the "Hide image" button in turn, but will not allow for random predefined images... I went with default-image to "fix" this issue now.

As a proof of concept you can test this issue in TwentySeventeen theme by modifying the code in inc/custom-header.php > twentyseventeen_custom_header_setup() function this way:

add_theme_support( 'custom-header', apply_filters( 'twentyseventeen_custom_header_args', array(
        // 'default-image'      => get_parent_theme_file_uri( '/assets/images/header.jpg' ),
        'random-default'   => true,
        'width'            => 2000,
        'height'           => 1200,
        'flex-height'      => true,
        'video'            => true,
        'wp-head-callback' => 'twentyseventeen_header_style',
) ) );

register_default_headers( array(
        'default-image' => array(
                'url'           => '%s/assets/images/header.jpg',
                'thumbnail_url' => '%s/assets/images/header.jpg',
                'description'   => __( 'Default Header Image', 'twentyseventeen' ),
        ),
        'espresso-image' => array(
                'url'           => '%s/assets/images/espresso.jpg',
                'thumbnail_url' => '%s/assets/images/espresso.jpg',
                'description'   => __( 'Espresso Header Image', 'twentyseventeen' ),
        ),
) );

Now when you don't do anything with customizer settings and just preview your website frontend, it will load the custom header images randomly. But it will not display "Hide image" button in customizer. Once you uncomment the default image part, the "Hide image" button in customizer is displayed, but randomization of header images stops on your website frontend.

So, to me this is an indication of custom header randomize functionality being broken in WordPress customizer. What do you think? Should I open a ticket or am I getting something wrong here?


5) [DISCUSSION NEEDED]

What exactly did you fix if you didn't understand?

I didn't understand the issue here, that's what I meant. There is no issue from my point of view and that's why I haven't fixed it previously and I haven't fixed it for now either.

...they were only showing in this theme, not other themes I have tested with the exact same menu.

Well, I'm sorry but I can't be blamed for other themes not implementing support for WordPress menu item description. Please understand that. I don't think my theme "can't figure out the menu system". Actually, I think the exact opposite and would go as far as saying the other themes can't as they don't apply this functionality ;)

According to WordPress menu codex page, the menu item description is purely left for themes to display or not:

The description will be displayed in the menu if the current theme supports it.

By default the wp_nav_menu() doesn't display menu item description, of course. But is introduction of support for this menu feature a bad practice? Is my theme implementation wrong? Is it restricted? I couldn't find any info about this restriction in theme requirements or guidelines. Should I really remove this functionality from the theme to pass the theme review? Is this really the first theme in WPORG repo with support for menu item description???

Regards,

Oliver

Last edited 7 weeks ago by webmandesign (previous) (diff)

#11 @themetracbot
7 weeks ago

  • Summary changed from THEME: Reykjavik – 0.9.3 to THEME: Reykjavik – 0.9.4

Reykjavik - 0.9.4

Reykjavik is fresh, lightweight, speed and SEO optimized, accessibility ready WordPress theme perfect for your next business, portfolio, blog or WooCommerce e-shop website. You can customize all elements of the theme to your needs. The theme works perfectly with Beaver Builder, Beaver Themer, Elementor, Visual Composer and other page builders to create fantastic layouts. It features mobile-optimized codebase and design with unique, easy-to-access mobile navigation. Build your website in no time with integrated one-click demo import functionality. Impress your website visitors with this beautiful theme now!

Theme URL - https://www.webmandesign.eu/portfolio/reykjavik-wordpress-theme/
Author URL - https://www.webmandesign.eu/

Trac Browser - https://themes.trac.wordpress.org/browser/reykjavik/0.9.4

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=reykjavik/0.9.3&new_path=reykjavik/0.9.4

History:

Ticket Summary Status Resolution Owner
#44682 THEME: Reykjavik – 0.9.4 closed closed-newer-version-uploaded alkesh7

(this ticket)

#46546 THEME: Reykjavik – 0.9.11 reviewing


https://themes.svn.wordpress.org/reykjavik/0.9.4/screenshot.jpg

#12 @webmandesign
7 weeks ago

Hi,

In theme version 0.9.4 I've fixed all errors thrown by NS Theme Check (Theme Sniffer) plugin.

Regards,

Oliver

#13 @joyously
7 weeks ago

22) [FIXED], but - This is actually an issue within WordPress itself I think

I don't think it's broken, but feel free to write a ticket.

5) There is no issue from my point of view and that's why I haven't fixed it previously and I haven't fixed it for now either.

Again, I never said it was broken, just that as a user, I don't like seeing extra text in my menu that I didn't put there. Most people don't want the extra (that's why it's the default), so most themes do not show it. It would be great to make it an option instead of just guessing which way to implement the menu.

Should I really remove this functionality from the theme to pass the theme review?

I never said that. I'll leave the review to @alkesh7.

#14 @alkesh7
7 weeks ago

Hi @joyously and @webmandesign,

From today or might be from tomorrow I will start reviewing this theme. And once it completed from my side then will sure provide next points if required.

Thanks!

#15 @webmandesign
6 weeks ago

Thank you, @alkesh7 !

#16 @alkesh7
6 weeks ago

Hi @webmandesign,

This is a report of my latest review.

1) Feature image only showing thumbnail with size - 150x150, https://i.imgur.com/C1ZHMwZ.jpg

2) Latest Posts with the pagination, page title should be different one - not site title with tagline - https://prnt.sc/gij2cw

3) Comments are not designed with the ordered list. Kindly, please check your whole theme again with default theme check xml file - https://codex.wordpress.org/Theme_Unit_Test

4) Color option missing with the page/post title or pagination header - https://i.imgur.com/wkr5r1u.jpg

5) There is no need to add "sk_Sk.mo" & "sk_Sk.po" files with the languages folder. We have https://translate.wordpress.org/ project for same.

6) Documentation folder not required within final theme folder. You can share your online documentation link with readme.txt file

7) Import Demo Data not working. If you are providing any third party plugin compatibility with your themes then must be in working condition.

Please complete this list now. There may be possibilities as I can find more points from my side in future with the theme.

#17 @webmandesign
6 weeks ago

Hi @alkesh7,

Thank you for your review!

Before I proceed with theme update, please provide more information on my questions/concerns:

  1. I'm quite confused about this one now. I've fixed the issue reported by @joyously in comment #7 above - point 18 and 23 specifically. There is a "Recommended image sizes" information added by the theme in media settings of admin area as I can not manipulate post thumbnails sizes and I can not stretch them with CSS. But your information now confuses me as you actually suggest something like that, or creating a new custom image size (please read my previous replies for explanation). Also, you say in the screenshot provided "based on accessibility-ready tag", can you point me to this requirement please? Have you already provided also accessibility review for the theme now, please? Finally, please check how this is in WordPress native themes, such as Twenty Seventeen - it's the same as in your screenshot of my theme, actually. So, is this really required to fix with not-really-needed additional custom image size?
  2. Again, I'm not quite sure what you mean? Isn't this left for theme design what is displayed there? Also, what exactly is wrong with what I display there in this theme? If you set a custom blog page in WP settings, the title will be inherited from that page. In default WP state what should I display there? Is there any WordPress requirement that says what I need to display there (please provide link)? The "Introduction" text is hidden accessibly and is provided for better explanation of that particular theme section. The theme actually contains several accessible texts simply to improve accessibility. I thought accessibility review will be provided by separate team after the standard review is finished, or am I mistaken? Have you also provided accessibility review now please?
  3. I see what you mean and will fix this.
  4. You can set these colors in Appearance > Customize > Theme Options > Intro. It works perfectly fine for me, do you experience some issues? Also, is it really required for theme to pass the review when not providing enough color options? Isn't this left for theme developer decision which options he/she provides? -> Just to make this clear for me for the future.
  5. I'm aware of WP translate project, but I still keep my translation files with the theme at least for time the theme is approved for WPORG repo. This simply means the WP translate engine will pick up the already provided Slovak translation immediately from the files I provide within the theme, thus skipping the translation approving process and saving time, and I can easily remove those files in later theme version then. So, is this really required to remove these files to pass the review?
  6. Is this really an issue please? Why can't I provide offline documentation for my theme users too? Should I really remove it?
  7. Import Demo Data is not working for now as I plan to make it work after I finish the theme demo website, which is obviously tightly connected also to how theme review goes. I will comment out the plugin recommendation for now so.

Thank you in advance for providing additional info!

Best regards,

Oliver

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


6 weeks ago

#19 @kevinhaig
6 weeks ago

@alkesh7 @webmandesign

1- 6 are recommendations and not requirements.....fix if you want.

At this point however, you can't provide a quick link to import demo data, that is not allowed. There is a security risk to allowing that, so it is not permitted.

#20 @alkesh7
6 weeks ago

Hello @webmandesign,

I have discussed about my points in our themereview community and we finalized as you only need to resolve number 7.

I will cross check your theme once again because you have lots of functionality with the theme and it may take more time to finalize everything.

Thanks!

#21 @webmandesign
5 weeks ago

Hi @alkesh7 and @kevinhaig,

Thank you for explanation. Please note that there is no way for me to tell what is recommendation and what is requirement in the list provided.

Although these are only recommendations mostly, they seems to be based on something. Please understand that I want to improve my themes all the time and make them according to WordPress theme requirements and recommendations. Could you please answer my questions or point me to the theme recommendations resources I might have missed?

Point 7:

@kevinhaig

At this point however, you can't provide a quick link to import demo data, that is not allowed. There is a security risk to allowing that, so it is not permitted.

I'm not sure what you mean. I'm not providing any quick link. I am making my theme compatible with One Click Demo Import plugin. According to theme review handbook I can do this. Can you please explain what is wrong with that? Should I remove the One Click Demo Import plugin compatibility from the theme altogether? If so, why exactly?
If there is a security risk, why One Click Demo Import is in WordPress.org plugins repo anyway?

@alkesh7

I have discussed about my points in our themereview community and we finalized as you only need to resolve number 7.

I'm not quite sure how should I resolve this now. You say above that if I provide plugin compatibility, it should work. But @kevinhaig says I can not include this due to security risk. I'm waiting for @kevinhaig explanation and will fix the point 7 then. Or do you have any suggestion on how to resolve this now?

The thing is I provide theme starter content in this theme, which is obviously not that comprehensive as the demo website of the theme (still in development). Besides, theme starter content only works on new WordPress installations and it does not work in WordPress network environment. Providing One Click Demo Import solution is the best way for theme users to start a new website easily in my experience.

Thank you for providing the additional information!

Best regards,

Oliver

#22 @kevinhaig
5 weeks ago

Sorry I had presumed that the link to setup the demo content was not conditional on the installation of a plugin.

I think what you have done with the demo content is fine because nothing happens until the user chooses to install the One Click Demo Import plugin. I'm not excited about the external links in the theme content....but OK.

I will however seek a more senior opinion, @greenshady , I know you are busy but are you OK with this?

@alkesh7 ..if you are otherwise happy with the theme go ahead and approve, I will look for it in the final review queue and pick it up for a more detailed check then.

#23 @alkesh7
5 weeks ago

  • Status changed from reviewing to approved

Thanks @kevinhaig for the update.

I am going to approve theme from my side. So, any other senior theme reviewer can double check this theme before approval.

Thanks,
Alkesh

#24 @alkesh7
5 weeks ago

Please keep in mind that I am facing that demo content xml file call with '404 not found'.

Screenshot - https://i.imgur.com/wr7HC0j.jpg

#25 follow-up: @greenshady
5 weeks ago

I'm good with both of these scenarios:

  • Theme provides demo content for One Click Demo Import plugin.
  • Theme imports the demo content on its own.

As long as the user knowingly and willingly makes the choice to import the content.

Ideally, the XML file would be packaged with the theme itself from a security standpoint. If it exists on another site, there's always the possibility that it could be an issue if that site got hacked.

#26 in reply to: ↑ 25 @kevinhaig
5 weeks ago

Replying to greenshady:

I'm good with both of these scenarios:

  • Theme provides demo content for One Click Demo Import plugin.
  • Theme imports the demo content on its own.

As long as the user knowingly and willingly makes the choice to import the content.

Ideally, the XML file would be packaged with the theme itself from a security standpoint. If it exists on another site, there's always the possibility that it could be an issue if that site got hacked.

I am not very excited about off site demo content being allowed on it's own with a one click update. It provides an avenue for mass distribution of spam or worse, and because WordPress.org hosts the theme leaves the user with a sense that everything has been checked and is fine.

If a plug in is used then it becomes a 2 step process. You can't accidentally or curiously click a button and whamo content is added. There is a purposeful intent with a plugin.

#27 @kevinhaig
5 weeks ago

Oops I see that this theme has the accessible ready tag.

@davidakennedy ....I will let you reopen the theme to do the accessible review.

#28 @webmandesign
5 weeks ago

Hi guys,

Thank you for feed back provided!

@alkesh7:

Thank you for the theme review and for approval, Alkesh!
I've went ahead and commented out all the One Click Demo Import plugin compatibility code in theme version 0.9.5 for now, until the theme demo content XML file is ready.

@kevinhaig and @greenshady:

Thanks for the input on demo content import.

Yes, I also think having a demo content XML packed with the theme would be better from security point of view, but please understand it's much easier for me to update (and potentially) fix the XML file immediately when I provide it remotely via my GitHub repository (it surely could be hacked too, but I presume chances are way smaller than having the file on some other servers).

I would never make theme demo content installable upon a single button click. It could really be dangerous as users may click it just by curiosity. Now you need to install One Click Demo Import plugin first and than click the import button, while you are surrounded by explanatory text on the import button screen. So, theme user is in full control here.

Also, if a theme user is really very concerned about potential security risk, he/she can simply use provided theme starter content instead, which is WordPress native feature, instead of the One Click Demo Import plugin one. And, still, one can just download the XML file from my GitHub repository and install it on their own - which is the same way as with WordPress theme unit test ;)

@davidakennedy:

Indeed, this theme still has to undergo an accessibility review :)

---

In theme version 0.9.5 I've improved styles and added missing WooCommerce styles, updated documentation, improved theme flexibility and PHP code a bit. I have also tested the theme with the new version of theme unit test and updated styles where needed.

Unfortunately, there was a new ticket created for uploaded theme version 0.9.5. Please see #46546

Thanks and best regards,

Oliver

#29 @webmandesign
3 weeks ago

@alkesh7, @kevinhaig, @greenshady, @davidakennedy

Hi guys,

Could you please give me update on the theme review process?

I'm afraid this theme might get forgotten in the queue as this ticket is obviously closed and the 0.9.5 theme update (ticket #46546) was registered as a new ticket with no previous review provided, so, the accessibility review team won't test it first probably.

Thanks!

Oliver

#30 @poena
2 weeks ago

  • Status changed from approved to reopened

Reopening and closing so that I can assign the latest version to myself for the final reviews.

Your theme is not forgotten, but there are several themes ahead if yours in the queue.

#31 @poena
2 weeks ago

  • Resolution set to closed-newer-version-uploaded
  • Status changed from reopened to closed

#32 @webmandesign
2 weeks ago

Hi @poena,

Great, thank you for the update! I was just worried as the theme update 0.9.5 was created as a new ticket and I thought the theme got buried in the queue somehow as it was approved already, but the new version (0.9.5) was shown as a new theme, not the previously reviewed one.

Thanks for your update again!

Regards,

Oliver

Note: See TracTickets for help on using tickets.