WordPress.org

Make WordPress Themes

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#27664 closed theme (live)

THEME: Handcraft Expo – 1.0.14

Reported by: _Y_Power Owned by: poena
Priority: new theme Keywords: theme-handcraft-expo
Cc: marco@…

Description

Handcraft Expo - 1.0.0

A WordPress Theme focused on graphical impact and simplicity: a traditional left sidebar spin-off helps you ‘streamline what really matters’. Geared towards personal or small-sized handcrafting businesses use. GNU GPL v3 guarantees all user’s freedoms.

Theme URL - http://handcraftexpo.nouveausiteweb.fr
Author URL - http://ypower.nouveausiteweb.fr

SVN - https://themes.svn.wordpress.org/handcraft-expo/1.0.0
ZIP - https://wordpress.org/themes/download/handcraft-expo.1.0.0.zip?nostats=1

History:

Ticket Summary Status Resolution Owner
#27664 THEME: Handcraft Expo – 1.0.14 closed live poena

(this ticket)

#31427 THEME: Handcraft Expo – 1.0.15 closed live themetracbot
#31505 THEME: Handcraft Expo – 1.0.16 closed live themetracbot
#31811 THEME: Handcraft Expo – 1.0.17 closed live themetracbot
#32206 THEME: Handcraft Expo – 1.0.18 closed live themetracbot
#32212 THEME: Handcraft Expo – 1.0.19 closed live themetracbot
#32308 THEME: Handcraft Expo – 1.0.20 closed live themetracbot
#32326 THEME: Handcraft Expo – 1.0.21 closed live themetracbot
#32587 THEME: Handcraft Expo – 1.0.22 closed live themetracbot
#32653 THEME: Handcraft Expo – 1.0.23 closed live themetracbot
#33762 THEME: Handcraft Expo – 1.0.24 closed live themetracbot
#33826 THEME: Handcraft Expo – 1.0.25 closed live themetracbot
#35299 THEME: Handcraft Expo – 1.0.26 closed live themetracbot
#36675 THEME: Handcraft Expo – 1.0.27 closed live themetracbot
#37855 THEME: Handcraft Expo – 1.0.28 closed live themetracbot
#39438 THEME: Handcraft Expo – 1.0.29 closed live themetracbot
#39520 THEME: Handcraft Expo – 1.0.30 closed live themetracbot
#40303 THEME: Handcraft Expo – 1.1.0 closed live themetracbot
#40411 THEME: Handcraft Expo – 1.1.1 closed live themetracbot
#42213 THEME: Handcraft Expo – 1.1.2 closed live themetracbot


https://themes.svn.wordpress.org/handcraft-expo/1.0.0/screenshot.png

Attachments (7)

handcraft-expo.zip (3.5 MB) - added by _Y_Power 2 years ago.
handcraft-expo 1.0.0 rev_1
repeating-nav.png (65.2 KB) - added by poena 2 years ago.
Repeating comment navigation, duplicated sidebar icon
default_load.JPG (156.4 KB) - added by kevinhaig 2 years ago.
responsive_issues.JPG (104.4 KB) - added by kevinhaig 2 years ago.
GIF.gif (283.6 KB) - added by poena 2 years ago.
Visual errors with top menu (no social links)
3navigations.png (13.3 KB) - added by poena 2 years ago.
GIF3.gif (1.0 MB) - added by poena 2 years ago.

Change History (69)

#1 @poena
2 years ago

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

#2 @poena
2 years ago

Hi!
Thank you for submitting your theme.
The theme has been reviewed against these guidelines: https://make.wordpress.org/themes/handbook/review/required/
This is only a partial review. Please fix the issues below and I will continue the review.
Please submit an update or reply within 7 days, or your theme will be closed as not-approved.

Note:
Your code is very difficult to read. It would be very helpful if you could reduce the multiple empty rows (sometimes up to 26 lines!) and the tabs (I counted 20+ tabs!) in your code.
See https://codex.wordpress.org/WordPress_Coding_Standards

Required:
Provide a unique prefix for everything the Theme defines in the public namespace, including options, functions, global variables, constants, post meta, etc.
You are using several different prefixes. The recommended prefix is your theme slug, handcraft-expo.

No PHP or JS errors.
I found the following errors:
Notice: Undefined variable: taxonomy in handcraft-expo\custom-tags.php on line 3
Notice: Trying to get property of non-object in handcraft-expo\custom-tags.php on line 3

Please test your theme and your images thoroughly. The images are not working for me.
get_template_directory_uri() should, in most cases, not be echoed. Only echo this if you want to print the actual link.

All untrusted data should be escaped before output.
You need to escape your get_theme_mod values on output. The files below are only examples, you need to escape all of them.
Examples:
In socials.php, both the urls and the image path should be escaped with esc_url(). See https://developer.wordpress.org/reference/functions/esc_url/
in SKETCHBOOK.php, use esc_attr() for the css. See https://developer.wordpress.org/reference/functions/esc_attr/
I'm also not sure why you are including this js in sketchbook.php instead of making a separate.js file?

Language
Your theme is not translation ready. There are texts that can't be translated, for example "Search results for", "Ooops! No content found!", "You might want to go back to the"...
You can read about how to translate themes here: https://developer.wordpress.org/themes/functionality/internationalization/

Last edited 2 years ago by poena (previous) (diff)

#3 @_Y_Power
2 years ago

Hi poena and thank you for your feedback!
I apologize for the hard-to-read code, I'll try to make it clearer.
The review is clear and detailed so I shouldn't have problems fixing the issues but I'll need some time to upload the fixed version since I won't unfortunately be able to work too much on it for the next two weeks: I'll anyway keep you updated and I'll try to upload some minor changes as well.

#4 @_Y_Power
2 years ago

Hi poena, just got back to work and hopefully by the end of next week I'll be able to upload a fixed version. In the meanwhile, I can apologize for including the file SKETCHBOOK.php since it's just a test file I left in the directory by accident... Uploading soon a version without it and minor fixes as well.

#5 @_Y_Power
2 years ago

Hi poena, uploading a new version that fixes:

Coding:
I made it more compact, hope it's going to work for you;

Prefixes
I've changed all functions, variables etc to handcraft-expo as you suggested except of course for php variables (no hyphen) for which I used the underscore: handcraft_expo. Let me know if that will work;

PHP errors
taxonomy in handcraft-expo\custom-tags.php on line 3 shouldn't give errors;

Untrusted data
being sanitizing all get_theme_mod calls and few others that I noticed;

Language
I've made the parts you pointed out translatable but I wasn't able to find anything else;

What I still couldn't find is a way to properly display images, as you pointed out: I'm assuming the default fallback for get_theme_mod is not working for some reason since I've verified that, once the options are saved to the database, everything works fine. Following the suggestions here:https://make.wordpress.org/themes/2014/07/09/using-sane-defaults-in-themes/ I've created a txt file in the 'dev' folder with all the options but I'm not really sure how to proceed. Am I overlooking something obvious? Could you please point me in the right direction?

@_Y_Power
2 years ago

handcraft-expo 1.0.0 rev_1

#6 @poena
2 years ago

Hi!
You need update the version number and use the uploader, we can't work with zip files because the uploader does some automated checks for us. https://wordpress.org/themes/getting-started/

#7 @themetracbot
2 years ago

  • Summary changed from THEME: Handcraft Expo – 1.0.0 to THEME: Handcraft Expo – 1.0.1

Handcraft Expo - 1.0.1

A WordPress Theme focused on graphical impact and simplicity: a traditional left sidebar spin-off helps you ‘streamline what really matters’. Geared towards personal or small-sized handcrafting businesses use. GNU GPL v3 guarantees all user’s freedoms.

Theme URL - http://handcraftexpo.nouveausiteweb.fr
Author URL - http://ypower.nouveausiteweb.fr

SVN - https://themes.svn.wordpress.org/handcraft-expo/1.0.1
ZIP - https://wordpress.org/themes/download/handcraft-expo.1.0.1.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=handcraft-expo/1.0.0&new_path=handcraft-expo/1.0.1

History:

Ticket Summary Status Resolution Owner
#27664 THEME: Handcraft Expo – 1.0.14 closed live poena

(this ticket)

#31427 THEME: Handcraft Expo – 1.0.15 closed live themetracbot
#31505 THEME: Handcraft Expo – 1.0.16 closed live themetracbot
#31811 THEME: Handcraft Expo – 1.0.17 closed live themetracbot
#32206 THEME: Handcraft Expo – 1.0.18 closed live themetracbot
#32212 THEME: Handcraft Expo – 1.0.19 closed live themetracbot
#32308 THEME: Handcraft Expo – 1.0.20 closed live themetracbot
#32326 THEME: Handcraft Expo – 1.0.21 closed live themetracbot
#32587 THEME: Handcraft Expo – 1.0.22 closed live themetracbot
#32653 THEME: Handcraft Expo – 1.0.23 closed live themetracbot
#33762 THEME: Handcraft Expo – 1.0.24 closed live themetracbot
#33826 THEME: Handcraft Expo – 1.0.25 closed live themetracbot
#35299 THEME: Handcraft Expo – 1.0.26 closed live themetracbot
#36675 THEME: Handcraft Expo – 1.0.27 closed live themetracbot
#37855 THEME: Handcraft Expo – 1.0.28 closed live themetracbot
#39438 THEME: Handcraft Expo – 1.0.29 closed live themetracbot
#39520 THEME: Handcraft Expo – 1.0.30 closed live themetracbot
#40303 THEME: Handcraft Expo – 1.1.0 closed live themetracbot
#40411 THEME: Handcraft Expo – 1.1.1 closed live themetracbot
#42213 THEME: Handcraft Expo – 1.1.2 closed live themetracbot


https://themes.svn.wordpress.org/handcraft-expo/1.0.1/screenshot.png

#8 @poena
2 years ago

Hi
I'm afraid I'm having problems testing your theme, like you said I don't think the default values are working.
There seem to be an additional problem with the customizer, because the customizer preview and the actual page looks very different from each other.

I think you need to simplify things to narrow down the problems. We are required to use WordPress functionality if it is available.
In this case I think it is about discovering what is already available, so right now I have more questions than actual requirements.

For example, this block of code is in your files 13 times. Have you looked into using wp_list_categories( $args );?https://codex.wordpress.org/Template_Tags/wp_list_categories

It would also reduce your own work load tremendously if had you put this inside a function, and then called the function when needed.

<?php 
	$handcraft_expo_categories = get_the_category();
	$handcraft_expo_separator = ', ';
	$handcraft_expo_categoriesOutput = '';
	if ($handcraft_expo_categories) { ?>
		 || Posted in:
<?php	
		foreach ($handcraft_expo_categories as $handcraft_expo_category) {
			$handcraft_expo_categoriesOutput .= '<a href="' . get_category_link($handcraft_expo_category->term_id) . '">' . $handcraft_expo_category->cat_name . '</a>' . $handcraft_expo_separator;
		}
		echo trim($categoriesOutput, $handcraft_expo_separator);
		}
?>	
$handcraft_expo_contentFooterCheck = get_theme_mod('handcraft-expo_content_footer_image');

is in your theme 16 times. I can't test the theme fully yet, so I can't actually see where this image is shown, but would it not be a lot easier to add this to footer.php?

Why are there css block in every php files? I believe this can be solved better with simple if else.. WordPress has several functions
to detect what page is being viewed, for example https://codex.wordpress.org/Function_Reference/is_page_template.

What changes are you making to the comments form, because I have never seen it being called this way. Are you making any other changes than editing the "reply" text?

When I try to view comments, I get the following error:

Notice: Undefined variable: handcraft_expo_CommentsArgs in \handcraft-expo\comments-special.php on line 82

Because "$handcraft_expo_CommentsArgs" does not exist in comments-special.php, only in comments.php.

Mean while, the only way to find the missing translations is really to look at each file. All the text needs to be translatable, including sidebar names and descriptions.
Example:

	register_sidebar( array(
			'name' =>'Widgets Mobile',
			'id' => 'widgets_mobile',
			'before_widget' => '<div class="widget-items">',
			'after_widget' => '</div>',
			'before_title' => '<h4>',
			'after_title' => '</h4>',
			));
			

Should be

	register_sidebar( array(
			'name' => esc_html__( 'Widgets Mobile' , 'handcraft-expo'),
			'id' => 'widgets_mobile',
			'before_widget' => '<div class="widget-items">',
			'after_widget' => '</div>',
			'before_title' => '<h4>',
			'after_title' => '</h4>',
			));
			

We use esc_htmlhere because the translation will be printed between the <h4> tags.

In handcraft-expo-blog-template.php, line 65

<article class="posts-meta">by <a href="<?php echo get_author_posts_url(get_the_author_meta('ID')); ?>"><?php the_author(); ?></a> on <?php the_time('F jS, Y'); ?> at <?php the_time('g:i a'); ?>.

You need to make "on" and "at" translatable.

#9 @poena
2 years ago

I also forgot to mention that the contact template needs to be removed. This kind of functionality is plugin territory and is not allowed in themes.

#10 @themetracbot
2 years ago

  • Summary changed from THEME: Handcraft Expo – 1.0.1 to THEME: Handcraft Expo – 1.0.2

Handcraft Expo - 1.0.2

A WordPress Theme focused on graphical impact and simplicity: a traditional left sidebar spin-off helps you &#8216;streamline what really matters&#8217;. Geared towards personal or small-sized handcrafting businesses use. GNU GPL v3 guarantees all user&#8217;s freedoms.

Theme URL - http://handcraftexpo.nouveausiteweb.fr
Author URL - http://ypower.nouveausiteweb.fr

SVN - https://themes.svn.wordpress.org/handcraft-expo/1.0.2
ZIP - https://wordpress.org/themes/download/handcraft-expo.1.0.2.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=handcraft-expo/1.0.1&new_path=handcraft-expo/1.0.2

History:

Ticket Summary Status Resolution Owner
#27664 THEME: Handcraft Expo – 1.0.14 closed live poena

(this ticket)

#31427 THEME: Handcraft Expo – 1.0.15 closed live themetracbot
#31505 THEME: Handcraft Expo – 1.0.16 closed live themetracbot
#31811 THEME: Handcraft Expo – 1.0.17 closed live themetracbot
#32206 THEME: Handcraft Expo – 1.0.18 closed live themetracbot
#32212 THEME: Handcraft Expo – 1.0.19 closed live themetracbot
#32308 THEME: Handcraft Expo – 1.0.20 closed live themetracbot
#32326 THEME: Handcraft Expo – 1.0.21 closed live themetracbot
#32587 THEME: Handcraft Expo – 1.0.22 closed live themetracbot
#32653 THEME: Handcraft Expo – 1.0.23 closed live themetracbot
#33762 THEME: Handcraft Expo – 1.0.24 closed live themetracbot
#33826 THEME: Handcraft Expo – 1.0.25 closed live themetracbot
#35299 THEME: Handcraft Expo – 1.0.26 closed live themetracbot
#36675 THEME: Handcraft Expo – 1.0.27 closed live themetracbot
#37855 THEME: Handcraft Expo – 1.0.28 closed live themetracbot
#39438 THEME: Handcraft Expo – 1.0.29 closed live themetracbot
#39520 THEME: Handcraft Expo – 1.0.30 closed live themetracbot
#40303 THEME: Handcraft Expo – 1.1.0 closed live themetracbot
#40411 THEME: Handcraft Expo – 1.1.1 closed live themetracbot
#42213 THEME: Handcraft Expo – 1.1.2 closed live themetracbot


https://themes.svn.wordpress.org/handcraft-expo/1.0.2/screenshot.png

#11 @_Y_Power
2 years ago

Hi poena and thank you very much for all the feedback and your quick replies!

I can almost hear you cry for a clearer code! Sorry about that. As you've seen, I tend to be reminded to stay DRY often and I understand the code can be hard to read for you but it just helps my own reading to stay a bit sparse: I'd like to optimize and minify everything at the end of the process but you will have to let me know if that is acceptable for you. In the meanwhile, I've put the code you pointed out in a function ( it does look better, now! ;) ) because it was certainly excessive. I certainly understand as well the requirement of using WordPress functions where possible.

The answers to your review and the update list:

DRY

as said above, function handcraft_expo_get_categories() is now used;

Footer image

$handcraft_expo_contentFooterCheck is referring to an image which will be displayed on the bottom right of posts/page: I did it this way since up to 4 widget areas are going to be displayed below it and I wouldn't know how to use the footer for it. Any suggestion is of course welcome.

CSS blocks

there are two kind of these CSS blocks: one is checking for the sidebar widget using is_active_sidebar() and the other is non-conditional (it will be used by choosing the template): I could get rid of the latter but not of the former and because of that I'd like to keep them together in the file since their functionalities are linked.

Comments

as you noticed, I use the comments-template to change the text (and possibly other things in the future) since I wanted the theme to ask the user to resize the window to be able to post a reply: I specified the unusual feature on the README file since I understand it can seem odd.

Errors

fixed the problem with the variable $handcraft_expo_CommentsArgs;

Translation

this turned out to be way harder than I expected: unchecked (un)translatable text seem to pop up from everywhere... Fixed all widgets as requested and I even found out the post-meta section you pointed out contained a 'by' to be fixed as well... please bear with me on this since it's kind of tricky.

Contact template

removed the template as requested;

I still couldn't find the way to solve the issue with the default database values but I verified that once the settings are changed and saved everything works correctly: still looking for the root cause.

#12 @poena
2 years ago

Hi!
I tested the theme on a fresh install but I'm still having problems getting the theme to work.

These php notices are still visible when I test the theme.

Notice: Undefined variable: taxonomy in handcraft-expo\custom-tags.php on line 2
Notice: Trying to get property of non-object in handcraft-expo\custom-tags.php on line 2

If they are not showing for you, it is probably due to different php versions, but we can't know what version the user has, so they need to be fixed.

The developer code reference is our best friend:
https://developer.wordpress.org/reference/functions/the_tags/
https://developer.wordpress.org/reference/functions/get_the_tag_list/
As you can see, the_tags() already uses get_the_tag_list, so you probably want to choose one of these functions, not both.
You are not calling get_the_tag_list correclty, and that is what is causing the notice. See https://codex.wordpress.org/Function_Reference/get_the_tag_list

When I try to view a child page, I get the following error:

Fatal error:  Call to undefined function handcraft_expo_get_post_ancestors() in handcraft-expo\functions.php on line 75

functions.php
What was your intention with this code?

$handcraft_expo_content = apply_filters( 'the_content', get_the_content() );
$handcraft_expo_content = str_replace( ']]>', ']]&gt;', $handcraft_expo_content );

This code causes a php error with a core file,
Notice: Trying to get property of non-object in \wordpress\wp-includes\post-template.php on line 279
and it prevented me from using the customizer and the media library.

Sanitizing customizer options:
You need to change some of your sanitizing functions.
'sanitize_callback' => 'esc_textarea', should only be used for text areas.
The range settings should be sanitized with intval, since they are all numbers.

Outputting customizer options in css:
For image links, use esc_url instead of esc_textarea.
For other options, use esc_attr.

Broken defaults
There are several get_theme_mods that are missing default values, and alot of empty css is printed.
Again, if the default values where in the stylesheets, you would only need to print this css if the option had actually been changed.

The logo should not have a default, since the end user would not be interested in using the themes default logo, they would want to use their own, instead it should be hidden if it is not used.

Missing defaults:
 1995: 	-webkit-transform: rotate(<?php echo esc_textarea(get_theme_mod('handcraft-expo_title_hover_rotate_effect')); ?>deg) 
 1996: 	-moz-transform: rotate(<?php echo esc_textarea(get_theme_mod('handcraft-expo_title_hover_rotate_effect')); ?>deg) 
 1997: 	transform: rotate(<?php echo esc_textarea(get_theme_mod('handcraft-expo_title_hover_rotate_effect')); ?>deg) 

 2018: scale(<?php echo esc_textarea(get_theme_mod('handcraft-expo_title_hover_scale_effect')); ?>);
 2019: scale(<?php echo esc_textarea(get_theme_mod('handcraft-expo_title_hover_scale_effect')); ?>);
 2020: scale(<?php echo esc_textarea(get_theme_mod('handcraft-expo_title_hover_scale_effect')); ?>);
 2021: scale(<?php echo esc_textarea(get_theme_mod('handcraft-expo_title_hover_scale_effect')); ?>);

All instances of:
 get_theme_mod('handcraft-expo_title_color'
 get_theme_mod('handcraft-expo_content_background_color'
 are missing defaults.

get_theme_mod('handcraft-expo_main_text_color' is missing default in 12 places.

There are more, I'm not able to list them all.

Errors in the css:

.col-xs-9 {background-image: url("  Notice:  Undefined variable: he_BackgroundDefault in handcraft-expo\functions.php on line 2156.

he_BackgroundDefault is used in two places in the css, but is not being set anywhere?

You don't need to handle the get_theme_mods like this if you are not using the variable in several places:

$handcraft_expo_pageTitlePosCheck = get_theme_mod('handcraft-expo_page_title_position', 'as_content');
if ($handcraft_expo_pageTitlePosCheck == 'as_content') { ?>

This would work just as well:

if ( get_theme_mod('handcraft-expo_page_title_position') == 'as_content') { ?>

If you want to put it in a variable, the default on the first line is not neccesary.

Please move the Media Queries to the stylesheet.


I also found this licensing issue for the gradient. It is not GPL compatible, so you will need to find a different sollution.

http://www.colorzilla.com/terms.html
"The copying, modification, revision, reproduction, republication, uploading, posting, transmission, or distribution for commercial non-personal purposes of any material or element from the Site including, but not limited to, the design or layout of the Site, individual elements of the Site design, or any logo or image without the express written permission of the Site Owner, or other owners of the Intellectual Property is strictly prohibited."

#13 @_Y_Power
2 years ago

Happy new year poena!

Thank you for pointing me in all the right directions. Here's the list of the fixes:

Errors

fixed the problem with 'the_tags()', which I didn't notice indeed due to different php versions: simplified the call and I can't see the errors anymore. The ancestors function is fixed as well as the '$he_BackgroundDefault' call in the customizer css output and the content_filter issue, which was a piece of code I left from a plugin I was working on.

Sanitizing

the 'intval' function didn't work for me: had a look here:https://codex.wordpress.org/Data_Validation and escaped the relevant bits with 'sanitize_text_field' instead. It works but I'm pretty sure it's not the proper way to do it... Meanwhile, all the get_theme_mod on outputs on css have now the proper data validation.

DRY

as you pointed out, removed all php variables which are not used at least twice and simplified the database settings calls.

GPL

hard to believe I overlooked the details on their licence but I did: thank you very much for looking at that. I just wanted to give a simple way to change the gradient but I'll eventually include another solution in an upgrade. Code is now standard css.

Misc

removed default logo fallback, moved media queries into stylesheet and fixed some styling issues.

Broken defaults

As you previously pointed out, there is a problem with the customizer not displaying correctly the page and that is related, as I understand it, to the order in which the 'customize_preview_init' function is executed. I've added a default css stylesheet which seems indeed to correct the issues but I'll need some time to find and fix all of them: now it's at least testable.

Thank you again for all the precious info.

#14 @themetracbot
2 years ago

  • Summary changed from THEME: Handcraft Expo – 1.0.2 to THEME: Handcraft Expo – 1.0.3

Handcraft Expo - 1.0.3

A WordPress Theme focused on graphical impact and simplicity: a traditional left sidebar spin-off helps you &#8216;streamline what really matters&#8217;. Geared towards personal or small-sized handcrafting businesses use. GNU GPL v3 guarantees all user&#8217;s freedoms.

Theme URL - http://handcraftexpo.nouveausiteweb.fr
Author URL - http://ypower.nouveausiteweb.fr

SVN - https://themes.svn.wordpress.org/handcraft-expo/1.0.3
ZIP - https://wordpress.org/themes/download/handcraft-expo.1.0.3.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=handcraft-expo/1.0.2&new_path=handcraft-expo/1.0.3

History:

Ticket Summary Status Resolution Owner
#27664 THEME: Handcraft Expo – 1.0.14 closed live poena

(this ticket)

#31427 THEME: Handcraft Expo – 1.0.15 closed live themetracbot
#31505 THEME: Handcraft Expo – 1.0.16 closed live themetracbot
#31811 THEME: Handcraft Expo – 1.0.17 closed live themetracbot
#32206 THEME: Handcraft Expo – 1.0.18 closed live themetracbot
#32212 THEME: Handcraft Expo – 1.0.19 closed live themetracbot
#32308 THEME: Handcraft Expo – 1.0.20 closed live themetracbot
#32326 THEME: Handcraft Expo – 1.0.21 closed live themetracbot
#32587 THEME: Handcraft Expo – 1.0.22 closed live themetracbot
#32653 THEME: Handcraft Expo – 1.0.23 closed live themetracbot
#33762 THEME: Handcraft Expo – 1.0.24 closed live themetracbot
#33826 THEME: Handcraft Expo – 1.0.25 closed live themetracbot
#35299 THEME: Handcraft Expo – 1.0.26 closed live themetracbot
#36675 THEME: Handcraft Expo – 1.0.27 closed live themetracbot
#37855 THEME: Handcraft Expo – 1.0.28 closed live themetracbot
#39438 THEME: Handcraft Expo – 1.0.29 closed live themetracbot
#39520 THEME: Handcraft Expo – 1.0.30 closed live themetracbot
#40303 THEME: Handcraft Expo – 1.1.0 closed live themetracbot
#40411 THEME: Handcraft Expo – 1.1.1 closed live themetracbot
#42213 THEME: Handcraft Expo – 1.1.2 closed live themetracbot


https://themes.svn.wordpress.org/handcraft-expo/1.0.3/screenshot.png

#15 @themetracbot
2 years ago

  • Summary changed from THEME: Handcraft Expo – 1.0.3 to THEME: Handcraft Expo – 1.0.4

Handcraft Expo - 1.0.4

A WordPress Theme focused on graphical impact and simplicity: a traditional left sidebar spin-off helps you &#8216;streamline what really matters&#8217;. Geared towards personal or small-sized handcrafting businesses use. GNU GPL v3 guarantees all user&#8217;s freedoms.

Theme URL - http://handcraftexpo.nouveausiteweb.fr
Author URL - http://ypower.nouveausiteweb.fr

SVN - https://themes.svn.wordpress.org/handcraft-expo/1.0.4
ZIP - https://wordpress.org/themes/download/handcraft-expo.1.0.4.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=handcraft-expo/1.0.3&new_path=handcraft-expo/1.0.4

History:

Ticket Summary Status Resolution Owner
#27664 THEME: Handcraft Expo – 1.0.14 closed live poena

(this ticket)

#31427 THEME: Handcraft Expo – 1.0.15 closed live themetracbot
#31505 THEME: Handcraft Expo – 1.0.16 closed live themetracbot
#31811 THEME: Handcraft Expo – 1.0.17 closed live themetracbot
#32206 THEME: Handcraft Expo – 1.0.18 closed live themetracbot
#32212 THEME: Handcraft Expo – 1.0.19 closed live themetracbot
#32308 THEME: Handcraft Expo – 1.0.20 closed live themetracbot
#32326 THEME: Handcraft Expo – 1.0.21 closed live themetracbot
#32587 THEME: Handcraft Expo – 1.0.22 closed live themetracbot
#32653 THEME: Handcraft Expo – 1.0.23 closed live themetracbot
#33762 THEME: Handcraft Expo – 1.0.24 closed live themetracbot
#33826 THEME: Handcraft Expo – 1.0.25 closed live themetracbot
#35299 THEME: Handcraft Expo – 1.0.26 closed live themetracbot
#36675 THEME: Handcraft Expo – 1.0.27 closed live themetracbot
#37855 THEME: Handcraft Expo – 1.0.28 closed live themetracbot
#39438 THEME: Handcraft Expo – 1.0.29 closed live themetracbot
#39520 THEME: Handcraft Expo – 1.0.30 closed live themetracbot
#40303 THEME: Handcraft Expo – 1.1.0 closed live themetracbot
#40411 THEME: Handcraft Expo – 1.1.1 closed live themetracbot
#42213 THEME: Handcraft Expo – 1.1.2 closed live themetracbot


https://themes.svn.wordpress.org/handcraft-expo/1.0.4/screenshot.png

#16 @_Y_Power
2 years ago

Hey poena,
sorry for the many uploads: this version should fix all the previous issues.

#17 @themetracbot
2 years ago

  • Summary changed from THEME: Handcraft Expo – 1.0.4 to THEME: Handcraft Expo – 1.0.5

Handcraft Expo - 1.0.5

A WordPress Theme focused on graphical impact and simplicity: a traditional left sidebar spin-off helps you &#8216;streamline what really matters&#8217;. Geared towards personal or small-sized handcrafting businesses use. GNU GPL v3 guarantees all user&#8217;s freedoms.

Theme URL - http://handcraftexpo.nouveausiteweb.fr
Author URL - http://ypower.nouveausiteweb.fr

SVN - https://themes.svn.wordpress.org/handcraft-expo/1.0.5
ZIP - https://wordpress.org/themes/download/handcraft-expo.1.0.5.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=handcraft-expo/1.0.4&new_path=handcraft-expo/1.0.5

History:

Ticket Summary Status Resolution Owner
#27664 THEME: Handcraft Expo – 1.0.14 closed live poena

(this ticket)

#31427 THEME: Handcraft Expo – 1.0.15 closed live themetracbot
#31505 THEME: Handcraft Expo – 1.0.16 closed live themetracbot
#31811 THEME: Handcraft Expo – 1.0.17 closed live themetracbot
#32206 THEME: Handcraft Expo – 1.0.18 closed live themetracbot
#32212 THEME: Handcraft Expo – 1.0.19 closed live themetracbot
#32308 THEME: Handcraft Expo – 1.0.20 closed live themetracbot
#32326 THEME: Handcraft Expo – 1.0.21 closed live themetracbot
#32587 THEME: Handcraft Expo – 1.0.22 closed live themetracbot
#32653 THEME: Handcraft Expo – 1.0.23 closed live themetracbot
#33762 THEME: Handcraft Expo – 1.0.24 closed live themetracbot
#33826 THEME: Handcraft Expo – 1.0.25 closed live themetracbot
#35299 THEME: Handcraft Expo – 1.0.26 closed live themetracbot
#36675 THEME: Handcraft Expo – 1.0.27 closed live themetracbot
#37855 THEME: Handcraft Expo – 1.0.28 closed live themetracbot
#39438 THEME: Handcraft Expo – 1.0.29 closed live themetracbot
#39520 THEME: Handcraft Expo – 1.0.30 closed live themetracbot
#40303 THEME: Handcraft Expo – 1.1.0 closed live themetracbot
#40411 THEME: Handcraft Expo – 1.1.1 closed live themetracbot
#42213 THEME: Handcraft Expo – 1.1.2 closed live themetracbot


https://themes.svn.wordpress.org/handcraft-expo/1.0.5/screenshot.png

#18 @_Y_Power
2 years ago

Hi poena,
I know I've made way too many uploads: newbies... ;)
This version should solve all the previous issues including all customizer's get_theme_mod defaults. You should now be able to continue the review.

#19 @poena
2 years ago

Hi!
Looks like all customizer options are working now, well done.

How do you mean that the user needs to resize the window to reply?
Using the default setup, the comment form works just fine?


Copying.txt has version 3 of GPL, and you are linking to version 3, but GNU Free Documentation License.txt, you have version 1.3.
Please choose one license.

Functions.php
Move all your add_theme_support and the register_nav_menus inside the handcraft_expo_setup()
function, to be added with after_setup_theme.
See: https://developer.wordpress.org/reference/functions/add_theme_support/

This is plugin territory, and needs to be removed:
Handcraft Expo Widgets shortcodes support
add_filter( 'widget_text', 'shortcode_unautop' );
add_filter( 'widget_text', 'do_shortcode' );

Header.php
These texts need to be made translatable:
line 86, Empowered by,

Single.php
Did you mean to allow differen't post formats?
Post formats are not active in the theme, but posts that were once saved with a post format
keeps their meta data, and that means that "if (get_post_format())" will return true.
When if (get_post_format()) returns true, the posts are not being displayed correctly
because the template parts are missing.
( line 71, get_template_part(get_post_format()); )

How to solve this really depends in if you want to use post formats or not.
If you don't want to use them, removing the If.. statement would allow the content
of those posts to display as regular posts.

If you want to use them and display them with different templates, well, then you need to
add the template parts.
To allow the creation of posts with different formats, you need to add them to functions.php,
inside handcraft_expo_setup().
Example:
add_theme_support( 'post-formats', array( 'aside', 'gallery' ) );

On line 37, $handcraft_expo_postFormatsCheck is not needed, because it is not used anywhere else?

To display the authors name and posts link, you can also use this function: the_author_posts_link();
line 43: <a href="<?php echo get_author_posts_url(get_the_author_meta('ID')); ?>"><?php the_author(); ?></a>

These texts need to be made translatable:
'Previous:', Next:'

comments.php:
When I view a single post that has more than one page of comments, the comment navigation is repeated, and the icon that allows us to show the right hand sidebar, is duplicated.
The first issue seem to be realted to foreach ($handcraft_expo_comments as $handcraft_expo_comment).

Please explain what you want to accomplish by using wp_link_pages here?
wp_link_pages is used to show different pages of a single post or page, it is normally used together with the_content().

mobile-html.php
On line 32, the image tag for the logo is printed, no matter if the logo is used or not.
These texts need to be made translatable:
'Related Pages:', 'Previous:', Next:', 'Previous Entries', 'Next Entries'

preview-html.php
I admit to finding the concept puzzling, why not use the actual menu item description here? (Each menu item can have a description, See Appearance, Menus)

Why is box 3 reserved for the latest posts? It is very confusing, especially if your first or second menu item has a sub menu.
I found that the sub menu that is displayed to the right of the main menu item, covered the preview box.

I recommend that you make a loop instead of repeating the code 8 times, but it is not required.
You should not use esc_textarea here, esc_textarea is for use inside a textarea.
If you don't want to allow html, use esc_html instead. See https://codex.wordpress.org/Data_Validation

This text need to be made translatable:
'describe your menu link here'

@poena
2 years ago

Repeating comment navigation, duplicated sidebar icon

#20 @themetracbot
2 years ago

  • Summary changed from THEME: Handcraft Expo – 1.0.5 to THEME: Handcraft Expo – 1.0.6

Handcraft Expo - 1.0.6

A WordPress Theme focused on graphical impact and simplicity: a traditional left sidebar spin-off helps you &#8216;streamline what really matters&#8217;. Geared towards personal or small-sized handcrafting businesses use. GNU GPL v3 guarantees all user&#8217;s freedoms.

Theme URL - http://handcraftexpo.nouveausiteweb.fr
Author URL - http://ypower.nouveausiteweb.fr

SVN - https://themes.svn.wordpress.org/handcraft-expo/1.0.6
ZIP - https://wordpress.org/themes/download/handcraft-expo.1.0.6.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=handcraft-expo/1.0.5&new_path=handcraft-expo/1.0.6

History:

Ticket Summary Status Resolution Owner
#27664 THEME: Handcraft Expo – 1.0.14 closed live poena

(this ticket)

#31427 THEME: Handcraft Expo – 1.0.15 closed live themetracbot
#31505 THEME: Handcraft Expo – 1.0.16 closed live themetracbot
#31811 THEME: Handcraft Expo – 1.0.17 closed live themetracbot
#32206 THEME: Handcraft Expo – 1.0.18 closed live themetracbot
#32212 THEME: Handcraft Expo – 1.0.19 closed live themetracbot
#32308 THEME: Handcraft Expo – 1.0.20 closed live themetracbot
#32326 THEME: Handcraft Expo – 1.0.21 closed live themetracbot
#32587 THEME: Handcraft Expo – 1.0.22 closed live themetracbot
#32653 THEME: Handcraft Expo – 1.0.23 closed live themetracbot
#33762 THEME: Handcraft Expo – 1.0.24 closed live themetracbot
#33826 THEME: Handcraft Expo – 1.0.25 closed live themetracbot
#35299 THEME: Handcraft Expo – 1.0.26 closed live themetracbot
#36675 THEME: Handcraft Expo – 1.0.27 closed live themetracbot
#37855 THEME: Handcraft Expo – 1.0.28 closed live themetracbot
#39438 THEME: Handcraft Expo – 1.0.29 closed live themetracbot
#39520 THEME: Handcraft Expo – 1.0.30 closed live themetracbot
#40303 THEME: Handcraft Expo – 1.1.0 closed live themetracbot
#40411 THEME: Handcraft Expo – 1.1.1 closed live themetracbot
#42213 THEME: Handcraft Expo – 1.1.2 closed live themetracbot


https://themes.svn.wordpress.org/handcraft-expo/1.0.6/screenshot.png

#21 @_Y_Power
2 years ago

Hey poena,
thank you as usual for all the great feedback!

Here's the list of changes according to your previous points:

functions.php
moved al theme support and navigation functions inside handcraft_expo_setup() and removed the shortcodes support.

header.php
made 'Empowered by' in line 86 translatable.

single.php

Did you mean to allow differen't post formats?

Yes, I initially wanted to include different post formats and then changed my mind but forgot to remove some parts: I now removed all the calls you pointed out as well some related CSS I found in functions.php. Than you for all the good info about post formats, they'll be handy in the future ;).
As you suggested, the_author_posts_link(); is now called and works like a charm.
Made navigation text translatable.

comments.php
You shouldn't be able to reply to comments when in full-window but only to post: the js script that automatically opens the comment's reply box shouldn't work.
I probably left the 'foreach' call (which was not needed and now removed) while I was working on the comments templates. I couldn't repeat your issue but I removed as well the wp_link_pages call and that should solve it. I can't really tell why the call was there in the first place: please understand I didn't have the chance to touch the theme since the first upload.

mobile-html.php
Fixed the 'img' tag issue.
Made the navigation translatable.

And now, for the 'controversies'.. :)

previewer-html
I completely understand you being 'puzzled' both conceptually and technically.
Trying to make a long story short, the original idea was what you pointed out: menu descriptions.
But then I wanted to give the options to upload pictures, change text and more so I thought about suggesting a plugin, which was tricky because it was really more complex to adapt something existing than write few lines of my own.
The choices of showcasing blog on third spot and a fixed structure instead of a prettier array are based, respectively, on ease of use and limiting needed resources: the choice of the textfield instead of the menu item description value is made on the assumption that the user would like to see the live preview.
I gave the X and Y offset options to place it anywhere in the content area.
I admit I myself think of the feature as a kind of plugin since a theme shouldn't ask for custom content and I respect your judgement on it: to me it really seems to fit the theme's philosophy and just about presentation.

GPL
I'm not sure I understood the requirements: the readme.txt is under the latest GNU Free Documentation License http://www.gnu.org/licenses/fdl-1.3.en.html since I assumed shouldn't be modyfied: should it all be under GPL?

#22 @poena
2 years ago

Hi!
I did not realize it was the FDL but no, it is not compatible with GPL. (It doesn't have to be GPL, only compatible)
I need some more time to look at your update, but this is what I am thinking right now:

I don't think that adding the post preview to the third menu item makes it easier to use.
While I can't require you to change it, as a user it is very confusing. Did you try a menu with submenus?
What if there are only two menu items?
It makes no sense to me to add the post preview to a different, unknown, menu item, in the middle of a menu.
Also, the post review has links that can't be accessed, as soon as the focus leaves the menu item, the preview box is hidden.

I still don't understand the comment reply problem. I think we are mixing up the terms :)
What do you mean with "full-window"? and "but only to post"
The comment reply is only visible when you are viewing the single post or page, but that's not from re-sizing the window.

I believe there is also some issues with the responsiveness. If I reduce the browser windows width or height, the menu changes, but when I increase it again, I have to refresh the page for the menu etc to fall back in place.

#23 @themetracbot
2 years ago

  • Summary changed from THEME: Handcraft Expo – 1.0.6 to THEME: Handcraft Expo – 1.0.7

Handcraft Expo - 1.0.7

A WordPress Theme focused on graphical impact and simplicity: a traditional left sidebar spin-off helps you &#8216;streamline what really matters&#8217;. Geared towards personal or small-sized handcrafting businesses use. GNU GPL v3 guarantees all user&#8217;s freedoms.

Theme URL - http://handcraftexpo.nouveausiteweb.fr
Author URL - http://ypower.nouveausiteweb.fr

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

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

History:

Ticket Summary Status Resolution Owner
#27664 THEME: Handcraft Expo – 1.0.14 closed live poena

(this ticket)

#31427 THEME: Handcraft Expo – 1.0.15 closed live themetracbot
#31505 THEME: Handcraft Expo – 1.0.16 closed live themetracbot
#31811 THEME: Handcraft Expo – 1.0.17 closed live themetracbot
#32206 THEME: Handcraft Expo – 1.0.18 closed live themetracbot
#32212 THEME: Handcraft Expo – 1.0.19 closed live themetracbot
#32308 THEME: Handcraft Expo – 1.0.20 closed live themetracbot
#32326 THEME: Handcraft Expo – 1.0.21 closed live themetracbot
#32587 THEME: Handcraft Expo – 1.0.22 closed live themetracbot
#32653 THEME: Handcraft Expo – 1.0.23 closed live themetracbot
#33762 THEME: Handcraft Expo – 1.0.24 closed live themetracbot
#33826 THEME: Handcraft Expo – 1.0.25 closed live themetracbot
#35299 THEME: Handcraft Expo – 1.0.26 closed live themetracbot
#36675 THEME: Handcraft Expo – 1.0.27 closed live themetracbot
#37855 THEME: Handcraft Expo – 1.0.28 closed live themetracbot
#39438 THEME: Handcraft Expo – 1.0.29 closed live themetracbot
#39520 THEME: Handcraft Expo – 1.0.30 closed live themetracbot
#40303 THEME: Handcraft Expo – 1.1.0 closed live themetracbot
#40411 THEME: Handcraft Expo – 1.1.1 closed live themetracbot
#42213 THEME: Handcraft Expo – 1.1.2 closed live themetracbot


https://themes.svn.wordpress.org/handcraft-expo/1.0.7/screenshot.png

#24 @_Y_Power
2 years ago

Hi poena,
your reply was quick!

Here's the list of the latest changes:

GPL
As you pointed out (and I did not know), FDL is not compatible with GPL: based on this page http://www.gnu.org/licenses/license-list.html#GPLCompatibleLicenses I then chose a GNU All-Permissive License.

previewer
Since I blindly trust your UX experience, I removed the 3rd spot block and added a dropdown menu to choose which one of the slots, if any, should show the latest post ;).

Responsiveness
I couldn't replicate the issue: may I ask you with which browser you experienced it?

Comments
Sorry for being too vague. The relevant js is comment_reply_link https://codex.wordpress.org/Function_Reference/comment_reply_link , which should be inhibithed when the theme shows its left sidebar (above h 768px).

#25 @poena
2 years ago

The responsive issue seems to be fixed now.
But the comment form is always visible for me in single view, as long as I am logged in, no matter the width of the window?

#26 @_Y_Power
2 years ago

Hey poena,
the comment form should be always visible: the difference should be in the behaviour of Resize the window to reply to comment link , which shouldn't open the reply to comment form just below the chosen comment (like the Reply link does).

Great to hear the responsive issue is gone: cold sweat for a while ;).

#27 @poena
2 years ago

  • Cc kevinhaig added

OK I finally understood what you meant by replying to a comment and not to a post (doh).
But there are still a couple of things that is making me scratch my head. I will ask a second reviewer to look at the theme with fresh eyes.
I am still trying to figure out why the sidebar sometimes gets duplicated.

I'm using chrome for this test.

The first time I enter or refresh a page, the submenu items are displayed on top of each other.
They don't go away until I hover over them.

When testing the pocket menu, it seems that the icon to fold down the menu is visible but unclickable until a certain width? This is confusing as I attempted to click it but nothing happened. Or perhaps the whole icon is not clickable?

I'm sorry but I am still having issues with the responsiveness. Sometimes I do need to refresh the page for the
regular menu to show again, and sometimes the WordPress admin bar is hidden. Sometimes the icon to fold down the pocket menu is actually under the admin bar.

Themes are not allowed to hide the admin bar, instead, adjust the position of the menu to below the admin bar if the user is logged in.

See the test install on http://www.plushie.se/.
At the bottom of the page is a post that has a gallery format. As you can see, it looks like there are some html problems.

And the very last post on the page has no title, and cannot be reached (you can solve this by adding an extra link to the post, for example when clicking on the date).

The div containing the post format breaks because class="" is duplicated:

<div class="<?php post_class(); ?>">

output:

<div class="class="post-1721 post type-post status-publish format-standard sticky hentry category-uncategorized"">

post_class(); does not need to and should not be used within class="".

https://codex.wordpress.org/Function_Reference/post_class

Valid html is not a requirement, but the theme should not be broken.

Recommended:

Test the theme with theme unit test: https://codex.wordpress.org/Theme_Unit_Test

If you go to http://www.plushie.se/uncategorized/gallery-x/
you can see how the images stretches outside the post container.


@kevinhaig Hi! Are you able to help us out here? I believe I'm becoming "blind to flaws at home" after reviewing the theme for five weeks. We need fresh eyes. Can you confirm if there are any issues with the responsiveness? Any other required issues?

(There are also a couple of things that both me and the author are aware of but that are not actually required by the guidelines, such as the empty css. )

#28 @kevinhaig
2 years ago

I will take a look at it later today :)

#29 @kevinhaig
2 years ago

Here are the issues I have picked up with a quick glance:

  • Submenus overlay primary menu elements on initial page load. This applies to the default menu fallback. When the menu is defined I am not getting that. But I am getting an annoying flash of the full menu before the final load is seen. This is likely a jQuery issue.
  • Left column is not properly responsive. When the screen narrows menu items expand below the content area and are not accessible. The footer credits also overlay the menu items in this case.
  • The sidebar does not work properly at all. If there is room for, it should show and not the activate button, in my opinion. When I put in the monster widget the widgets extend below the sidebar content area and are not accessible.
  • Many of the issues are design in nature but effectively break the page. So in my opinion it becomes a requirement they be fixed before being approved. In all honesty the author should want to fix these issues anyways.
  • I find it very interesting that the vertical scroll bar does not affect the menu column or the sidebar. Content below the screen is not accessible in these areas.
  • I have attached a few screenshots, to help show what I am seeing.
  • The css styling has not been properly set for images as the image aspect ratio is not maintained.
  • I also don't understand why there are four Bottom widgets when one would do fine with proper css. The widgets are placed in one column anyway.

Other issues

  • Remove <title><?php wp_title(''); ?> </titlefrom header.php, you do not need it.
  • The preview image is CC By 2.0 and that is not GPL compatible, it must be replaced.
  • Resize the window to reply to comment should be translated and it does not work. A simple Reply link would be better.

#30 @poena
2 years ago

Thank you Kevin!

@_Y_Power Reminds me: the screenshot needs to look more like the theme when it is first loaded with the default settings. The recommended screenshot sizes are 880x600 or 1200x900.

#31 @_Y_Power
2 years ago

Hi @poena and @kevinhaig,
thank you very much for all your feedback!

Looks like I have lots of work to do: please allow me some time to fix all the above issues.

#32 @themetracbot
2 years ago

  • Summary changed from THEME: Handcraft Expo – 1.0.7 to THEME: Handcraft Expo – 1.0.8

Handcraft Expo - 1.0.8

A WordPress Theme focused on graphical impact and simplicity: a traditional left sidebar spin-off helps you &#8216;streamline what really matters&#8217;. Geared towards personal or small-sized handcrafting businesses use. GNU GPL v3 guarantees all user&#8217;s freedoms.

Theme URL - http://handcraftexpo.nouveausiteweb.fr
Author URL - http://ypower.nouveausiteweb.fr

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

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

History:

Ticket Summary Status Resolution Owner
#27664 THEME: Handcraft Expo – 1.0.14 closed live poena

(this ticket)

#31427 THEME: Handcraft Expo – 1.0.15 closed live themetracbot
#31505 THEME: Handcraft Expo – 1.0.16 closed live themetracbot
#31811 THEME: Handcraft Expo – 1.0.17 closed live themetracbot
#32206 THEME: Handcraft Expo – 1.0.18 closed live themetracbot
#32212 THEME: Handcraft Expo – 1.0.19 closed live themetracbot
#32308 THEME: Handcraft Expo – 1.0.20 closed live themetracbot
#32326 THEME: Handcraft Expo – 1.0.21 closed live themetracbot
#32587 THEME: Handcraft Expo – 1.0.22 closed live themetracbot
#32653 THEME: Handcraft Expo – 1.0.23 closed live themetracbot
#33762 THEME: Handcraft Expo – 1.0.24 closed live themetracbot
#33826 THEME: Handcraft Expo – 1.0.25 closed live themetracbot
#35299 THEME: Handcraft Expo – 1.0.26 closed live themetracbot
#36675 THEME: Handcraft Expo – 1.0.27 closed live themetracbot
#37855 THEME: Handcraft Expo – 1.0.28 closed live themetracbot
#39438 THEME: Handcraft Expo – 1.0.29 closed live themetracbot
#39520 THEME: Handcraft Expo – 1.0.30 closed live themetracbot
#40303 THEME: Handcraft Expo – 1.1.0 closed live themetracbot
#40411 THEME: Handcraft Expo – 1.1.1 closed live themetracbot
#42213 THEME: Handcraft Expo – 1.1.2 closed live themetracbot


https://themes.svn.wordpress.org/handcraft-expo/1.0.8/screenshot.png

#33 @_Y_Power
2 years ago

Hey @poena and @kevinhaig,
thanks again for all the feedback. I myself found many issues with the theme in addition to the ones you pointed out but essentially related to the same things and I ended up rearranging elements quite a bit, hopefully fixing all the issues. I also added a 'Your theme' section in the customizer, which the theme check plugin warns be about since there are hard-coded links in it: did I do it correctly?

Here's the list of changes:

(@poena)

Poket Menu
Now using 'is_user_logged_in() to solve the admin bar layout issue and fixed minor issues with the navigation as well.

Menus
Before selecting menus, the theme will now display a list of all its sub-menus items: I added an 'important' note in the readme file about it.

HTML
Fixed the post_class issue.
I haven't been able to replicate the image overflow issue but I hope it will be fixed by the CSS changes I made on images (see below).

Screenshot
Changed the screenshot and default colors to match: differences with defaults are logo, background and previewer pictures (which I'd like to leave empty by default), selected menu and activated widgets sidebar. Is it ok?

(@kevinhaig)

Flash before load

Submenus overlay primary menu elements on initial page load. This applies to the default menu fallback. When the menu is defined I am not getting that. But I am getting an annoying flash of the full menu before the final load is seen. This is likely a jQuery issue.

Fixed the former issue (see above) but couldn't fix the latter: since I can't see it on actual sites, I do believe indeed to be related to jQuery.

Content scroll

Many of the issues are design in nature but effectively break the page. So in my opinion it becomes a requirement they be fixed before being approved. In all honesty the author should want to fix these issues anyways.

All sidebars are now showing their content on vertical scroll. I basically just added a overflow-y: auto and adapted the surronding code: I originally did not want to do that but I agreed with what you said about effectively breaking the theme and changed my mind about it.

Images
Fixed the images proportion issue by addressing the CSS height values.

Bottom Widgets
Reduced them to 2: I added lots of them because I naturally tend to do so... :)

Others
Removed the title tag and substituted the picture with one with the proper license (CC4).
Changed Resize the window to reply to comment to Click and resize the window to reply to comment and made it translatable. The link does work but the user needs to resize the window to be able to see the corresponding reply-to-comment box.


The only thing I did not change is the default 'hide()' state of the widgets sidebar, which only shows the 'switch' once activated: I'm not resisting any critique here but instead I'd like to ask both of you (and possibly anyone else) an opinion on it. I really understand that as a general rule of thumb the idea of hiding it is wrong but what I'm trying to achieve with the theme layout is to invite to read, or 'discover', the content from left to right, hence the choice. As I said, I'm agnostic about it and I really value your feedback.

#34 @poena
2 years ago

Hi!
The pocket menu is still partly hidden when I am logged in and viewing a page or post.
It behaves differently on different pages, when I am on the front page, the WordPress admin bar is fully hidden, and that is not allowed.

I found the following js error:
Failed to load resource: the server responded with a status of 404 (Not Found) for handcraft-expo/js/mobile-logged-user?ver=1

You forgot to add the file ending in functions.php line 19.

I don't think that adding two new scripts is the solution here, are they really needed?
You are using abosulute positioning, so you could use css and the WordPress function is_admin_bar_showing() to place your menu.

	 if ( is_admin_bar_showing() ) {
	 	?>
	 	.main-navigation{top:32px;}

	 	@media screen and ( max-width: 782px ) {
			.main-navigation{top:46px;}
		}
	<?php
	 }

(On 782 width, the admin bar changes height).

There are still html errors in the post listing:

<div id="post-1749" class="post-1749 post type-post status-publish format-gallery hentry category-uncategorized post_format-post-format-gallery" <="" div=""><p></p>
					
</div>

The screenshot is going to look very strange in the directory and in the install with the white part over it.
It should be in a 4:3 aspect ratio. The part where the theme name and slogan is written is not part of the theme, the screenshot should look like the theme does when it is first installed.

Note:
Each widget area (sidebar) can hold any number of widgets, that is why multiple sidebars next to each other are not needed, unless you want to give them different colors or similar.

Last edited 2 years ago by poena (previous) (diff)

#35 @_Y_Power
2 years ago

Hi @poena ,
thank you for the review and all the tips: I'm going to fix everything asap.
One question: I created the .pot file and translated the theme in two other languages (french and italian for the moment) in .po files. Should I include them in the next upload or is there another procedure?

#36 @poena
2 years ago

Hi
They should be included in the upload.
Create a folder called languages and add them to the folder.

#37 @themetracbot
2 years ago

  • Summary changed from THEME: Handcraft Expo – 1.0.8 to THEME: Handcraft Expo – 1.0.9

Handcraft Expo - 1.0.9

A WordPress Theme focused on graphical impact and simplicity: a traditional left sidebar spin-off helps you &#8216;streamline what really matters&#8217;. Geared towards personal or small-sized handcrafting businesses use. GNU GPL v3 guarantees all user&#8217;s freedoms. Italian translation included.

Theme URL - http://handcraftexpo.nouveausiteweb.fr
Author URL - http://ypower.nouveausiteweb.fr

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

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

History:

Ticket Summary Status Resolution Owner
#27664 THEME: Handcraft Expo – 1.0.14 closed live poena

(this ticket)

#31427 THEME: Handcraft Expo – 1.0.15 closed live themetracbot
#31505 THEME: Handcraft Expo – 1.0.16 closed live themetracbot
#31811 THEME: Handcraft Expo – 1.0.17 closed live themetracbot
#32206 THEME: Handcraft Expo – 1.0.18 closed live themetracbot
#32212 THEME: Handcraft Expo – 1.0.19 closed live themetracbot
#32308 THEME: Handcraft Expo – 1.0.20 closed live themetracbot
#32326 THEME: Handcraft Expo – 1.0.21 closed live themetracbot
#32587 THEME: Handcraft Expo – 1.0.22 closed live themetracbot
#32653 THEME: Handcraft Expo – 1.0.23 closed live themetracbot
#33762 THEME: Handcraft Expo – 1.0.24 closed live themetracbot
#33826 THEME: Handcraft Expo – 1.0.25 closed live themetracbot
#35299 THEME: Handcraft Expo – 1.0.26 closed live themetracbot
#36675 THEME: Handcraft Expo – 1.0.27 closed live themetracbot
#37855 THEME: Handcraft Expo – 1.0.28 closed live themetracbot
#39438 THEME: Handcraft Expo – 1.0.29 closed live themetracbot
#39520 THEME: Handcraft Expo – 1.0.30 closed live themetracbot
#40303 THEME: Handcraft Expo – 1.1.0 closed live themetracbot
#40411 THEME: Handcraft Expo – 1.1.1 closed live themetracbot
#42213 THEME: Handcraft Expo – 1.1.2 closed live themetracbot


https://themes.svn.wordpress.org/handcraft-expo/1.0.9/screenshot.png

#38 @_Y_Power
2 years ago

Hi @poena ,

the latest changes according to your points:

Pocket Menu responsiveness
Hope to have fixed all the issues by basically copying your code: anther script was certainly not needed and I didn't use is_admin_bar_showing() just because I didn't know it existed so thank you for that! :)

html
Should have fixed the breaking by having added the proper ending tags (doh...).

Screenshot
Changed once again: hopefully will look good enough!

Widgets
Got the tip and removed the second bottom widget.

Translations
Added both .pot and .po in the languages folder: just italian for the moment as I didn't have the time to finish it quickly. I'll add it soon.


Also put the previewer html in a conditional function and added edit_post_link() at the end of content.

Thank you again

#39 @poena
2 years ago

  • Cc kevinhaig removed

Hi
This works better, but if no pocket menu is selected, the default menu is always visible, it can't be closed.

Social icons (socials.php)

Required:
When a custom link is added in the customizer, but no image is selected, the output is broken:

<div class="social-icons">
<a id="custom-site-1" href="https://www.yoursite1.com"><img src="" /></a>
</div>

You can sort this by adding a default image, Lines 79 and forward:

<img src="<?php echo esc_url(get_theme_mod('handcraft-expo_social_custom_1_img', ' missing default here ')); ?>" />

Recommended:
But also consider not printing the div(s) if there are no links to show.
In mobile-html.php line 6, the <div class="mobile-social-icons"></div> is printed even if the social links have been moved to a different position.


page.php

Required:
These texts needs to be made translatable:
Line 34, Main page:,
Line 39, Related Pages:,
Line 69, 75: Next and Previous


mobile-html.php

Required:
On lines 146 and 151, the words Next and Previous needs to be made translatable.

I belive you are trying to use wp_link_pages to display the next and previous page, but if that worked, it would mean the page navigation would be duplicated?
wp_link_pages() is used to show different pages of a paginated post or page and should not be used here. See https://codex.wordpress.org/Function_Reference/wp_link_pages
WordPress does not have a function for displaying next and previous pages.

Recommended:
It is possible that I misunderstood the style blocks, as we have discussed them before:
But mobile-main-title only seem to be used in one place in the theme, so it doesn't need any extra styling depending on this option?
If it doesn't need styling depending on the option, it should be moved to the base stylesheet (and the same goes for style="text-decoration: none;" for the link).
The <style type="text/css"> is not valid html when placed inside a div. So if you don't want to move it to stylesheet, please change it to inline css.

<?php 
	if (get_theme_mod('handcraft-expo_title_show', '1')) { ?>
			<style type="text/css">
				.mobile-main-title {
					padding-top: 15%;
				}
			</style>
			<div class="mobile-main-title">
				<a href="<?php echo home_url(); ?>" style="text-decoration: none;"><h1 class="mobile-main-title-show"><?php bloginfo('title'); ?></h1>
				<h3 class="mobile-main-tagline-show"><?php bloginfo('description'); ?></h3></a>
			</div>
<?php };


<?php 
	if (get_theme_mod('handcraft-expo_title_show', '1')) { ?>
			<div class="mobile-main-title" style="padding-top:15%">
				<a href="<?php echo home_url(); ?>" style="text-decoration: none;"><h1 class="mobile-main-title-show"><?php bloginfo('title'); ?></h1>
				<h3 class="mobile-main-tagline-show"><?php bloginfo('description'); ?></h3></a>
			</div>
<?php };


On lines 117-128, the style has already been added to style.css so it can be removed. Please check if there are blocks like these that are duplicated.

On lines 295 and forward, and also in archive.php, I recommend using the_archive_title() instead.
See https://developer.wordpress.org/reference/functions/the_archive_title/

Placing a div inside a p is also not valid html. Here are some explainations to why it breaks: http://stackoverflow.com/questions/8397852/why-p-tag-cant-contain-div-tag-inside-it.

9 matches across 4 files:
themes\handcraft-expo\functions.php:
   96: 			<p><div class="<?php post_class(); ?>"><?php the_content(); ?></div></p>	
themes\handcraft-expo\handcraft-expo-blog-template.php:
   78: 		   <p><div id="post-<?php the_ID(); ?>" <?php post_class(); ?>>
themes\handcraft-expo\mobile-html.php:
   68: 				<p><div <?php post_class(); ?>><?php the_content(); ?></div></p>							
   88: 				<p><div <?php post_class(); ?>><?php the_excerpt(); ?></div></p>
  202: 			<p><div <?php post_class(); ?>><?php the_content(); ?></div></p>
  246: 			<p><div <?php post_class(); ?>><?php the_content(); ?></div></p>
  330: 				<p><div <?php post_class(); ?>><?php the_excerpt(); ?></div></p>								
  369: 				<p><div <?php post_class(); ?>><?php the_excerpt(); ?></div></p>								

themes\handcraft-expo\single.php:
   51: 		<p><div <?php post_class(); ?>><?php the_content(); ?></div></p>

Templates

Recommended:
Place templates in a separate folder.

When a template is used, the name is added as a class to the body tag. Example:

<body class="page page-id-703 page-template page-template-handcraft-expo-2-columns-template page-template-handcraft-expo-2-columns-template-php logged-in admin-bar no-customize-support" >

This makes it easy to style content of templates in the base stylesheet.
I'm using one template as example, but it goes for all of them:

.mobile-page-content .pages-navigation

would be

.page-template-handcraft-expo-2-columns-template .mobile-page-content .pages-navigation

(As you can see it also makes sense to shorten the names of the templates)

The following classes are also added to the body for archives, so they can be targeted in the same way:

archive category category-(name) category-(id) 
archive tag tag-(name) tag-(id)

comments.php and comments-special.php

Required
Move if ( is_singular() ) wp_enqueue_script( "comment-reply" ); into functions.php, handcraft_expo_resources() (Do not duplicate it, but remove it from the comment files).

Question:
Why do you need wp_reset_postdata(); here? It looks like it is already run after your custom queries for the pagination? https://codex.wordpress.org/Function_Reference/wp_reset_postdata


bottom-widgets.php

Required
You have removed some widgets from the settings, but in this file, the codes for widgets_bar_bottom_2 to 4 is still there. Please remove them.

#40 @_Y_Power
2 years ago

Hey @poena !
Wow, thank you as usual for all the clues, lots of work to do: I'll jump on it asap.

#41 @themetracbot
2 years ago

  • Summary changed from THEME: Handcraft Expo – 1.0.9 to THEME: Handcraft Expo – 1.0.10

Handcraft Expo - 1.0.10

A WordPress Theme focused on graphical impact and simplicity: a traditional left sidebar spin-off helps you &#8216;streamline what really matters&#8217;. Geared towards personal or small-sized handcrafting businesses use. GNU GPL v3 guarantees all user&#8217;s freedoms. Italian translation included.

Theme URL - http://handcraftexpo.nouveausiteweb.fr
Author URL - http://ypower.nouveausiteweb.fr

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

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

History:

Ticket Summary Status Resolution Owner
#27664 THEME: Handcraft Expo – 1.0.14 closed live poena

(this ticket)

#31427 THEME: Handcraft Expo – 1.0.15 closed live themetracbot
#31505 THEME: Handcraft Expo – 1.0.16 closed live themetracbot
#31811 THEME: Handcraft Expo – 1.0.17 closed live themetracbot
#32206 THEME: Handcraft Expo – 1.0.18 closed live themetracbot
#32212 THEME: Handcraft Expo – 1.0.19 closed live themetracbot
#32308 THEME: Handcraft Expo – 1.0.20 closed live themetracbot
#32326 THEME: Handcraft Expo – 1.0.21 closed live themetracbot
#32587 THEME: Handcraft Expo – 1.0.22 closed live themetracbot
#32653 THEME: Handcraft Expo – 1.0.23 closed live themetracbot
#33762 THEME: Handcraft Expo – 1.0.24 closed live themetracbot
#33826 THEME: Handcraft Expo – 1.0.25 closed live themetracbot
#35299 THEME: Handcraft Expo – 1.0.26 closed live themetracbot
#36675 THEME: Handcraft Expo – 1.0.27 closed live themetracbot
#37855 THEME: Handcraft Expo – 1.0.28 closed live themetracbot
#39438 THEME: Handcraft Expo – 1.0.29 closed live themetracbot
#39520 THEME: Handcraft Expo – 1.0.30 closed live themetracbot
#40303 THEME: Handcraft Expo – 1.1.0 closed live themetracbot
#40411 THEME: Handcraft Expo – 1.1.1 closed live themetracbot
#42213 THEME: Handcraft Expo – 1.1.2 closed live themetracbot


https://themes.svn.wordpress.org/handcraft-expo/1.0.10/screenshot.png

#42 @_Y_Power
2 years ago

Hi @poena ,

I had a better look at the codex and realized that you have been trying to address me on the css blocks issue in the proper way since the beginning of the review but I couldn't - due to ignorance on my part - understand what you were referring to. I really appreciate your patience.

Here's the list of the fixes:

Pocket Menu
Should now be possible to close the dropdown menu when no menu has been assigned.

Social icons
Created a default image as you suggested but the social icons div is always printed since I use it for styling.

page.php
Made requested translations.

mobile-html.php
Made requested translations.

I might have (of course) misunderstood something but wp_link_pages() is used with 'is_page()' or 'is_single' conditionals and they shouldn't ever be displayed together?

Removed all css blocks in templates and other files and enqueued styles with conditionals in functions.php: not sure it's the best way but it's certainly way better than before.
Left two inline css. Thank you for all the info about page-template generated classes.

Using the_archive_title() (also in 'archive.php') as you suggested.

Removed ending 'p' tags: I did not know it would break the html. Interesting comment from the OP of the link you sent: 'I know that nobody will write code like the second one. I just want know why.' :D

Templates
Moved all templates inside folder and removed all css blocks.
Did not change names lenght but planning to do so in later versions since it really does make sense.

comments.php and comments-special.php
Not sure why I used 'wp_reset_postdata()': I must have thought it was needed because of 'paginate_comments_links()' but I removed it and it seems to work just fine.
Moved the comment-reply request as requested.

bottom-widgets.php
Removed the unnecessary code.


Updated '.pot' and added french translation.

@poena
2 years ago

Visual errors with top menu (no social links)

@poena
2 years ago

#43 @poena
2 years ago

Hi
Most of the problems I mentioned where fixed, but lets look at the navigation.

There are still a couple of places where the next and previous navigation is not translatable, it proved to be too difficult to find all the texts in one review (or 7 ) :).
archive.php line 26
mobile-html.php lines 198, 199, 328, 367
search.php line 38

I noticed that when there is only one or two pages (-like on a new fresh WordPress install),
the page navigation is not working because the offset is not possible. I get this php error:

Notice: Undefined offset: -1 in themes\handcraft-expo\page.php on line 51
Notice: Undefined offset: 1 in themes\handcraft-expo\page.php on line 52

In comments.php and comments-special.php I don't think $handcraft_expo_Comments and $handcraft_expo_comments are used and they should be removed.

Recommended:

I can now close the pocket menu, but if there are no social links visible, the menu is not high enough for the full icon to show, see gif. Note: The menu is also open by default.

When you create a new post or page, you can split the content into different parts in the editor by inserting a page break (Shift Alt P).
To display the content of a post or page, you use the_content() (or the_excerpt() for excerpts),
but the_content() will only display the first part, not page 2-3 and so on.

To display links to the other parts of the same post or page, you use wp_link_pages().
It does not link to the next post or to the next "page item", only to the next part of the same post or page.

You are using it correctly in single.php, but in page.php and mobile-html.php it needs to be moved so that it is above the other navigation:
content
navigation to part 2 of the content
navigation to next and previous page

It is confusing when you have tripple navigation in one page, I think it needs to be separated, perhaps by a headline or different design. See screenshot.
navigation to part 2 of the content
navigation to next and previous page
navigation to next and previous page of comments

I would not recommend enqueuing very small stylesheets that are only 4-5 lines, you could add it to handcraft_expo_customize_css_output.

You can also add a check to see if the sidebar is active, then add a new class to body and style the classes in the main stylesheet. Examples:

functions.php:

add_filter( 'body_class', 'handcraft_expo_classes' );
function handcraft_expo_classes($classes) {
	 /* 	
	 *		Is the sidebar active? Add 'active-sidebar' to the $classes array.
	 */		
	if ( is_active_sidebar('widgets_sidebar') ) {
		$classes[] = 'active-sidebar';
	}elseif (! is_active_sidebar('widgets_sidebar') ) {
		$classes[] = 'no-sidebar';
	}
	return $classes;
}

style.css:

.active-sidebar .page-main-content {
	min-width: 88%;
	max-width: 88%;
}

There is some css in handcraft_expo_customize_css_output that can be shortened, strive to print as little css here as possible ( Without enqueuing tiny styles :) ).

Instead of opening and closing a stylesheet tag multiple times, you can use one tag to hold all the styles.

On line 2066 and 2588, 2749, 2761, 2813, 2868, the transition and opacity is not affected by the option, only the color is, so the rest can be moved to the main stylesheet.
On line 2200, the default style should be moved to the main style sheet, to be written when the option is changed.


In handcraft_expo_GetPreviewerPost I recommend that you remove the links, because they aren't clickable and it is very tempting to try to click them instead of the menu item :)

#44 @themetracbot
2 years ago

  • Summary changed from THEME: Handcraft Expo – 1.0.10 to THEME: Handcraft Expo – 1.0.11

Handcraft Expo - 1.0.11

A WordPress Theme focused on graphical impact and simplicity: a traditional left sidebar spin-off helps you &#8216;streamline what really matters&#8217;. Geared towards personal or small-sized handcrafting businesses use. GNU GPL v3 guarantees all user&#8217;s freedoms. French and Italian translations included.

Theme URL - http://handcraftexpo.nouveausiteweb.fr
Author URL - http://ypower.nouveausiteweb.fr

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

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

History:

Ticket Summary Status Resolution Owner
#27664 THEME: Handcraft Expo – 1.0.14 closed live poena

(this ticket)

#31427 THEME: Handcraft Expo – 1.0.15 closed live themetracbot
#31505 THEME: Handcraft Expo – 1.0.16 closed live themetracbot
#31811 THEME: Handcraft Expo – 1.0.17 closed live themetracbot
#32206 THEME: Handcraft Expo – 1.0.18 closed live themetracbot
#32212 THEME: Handcraft Expo – 1.0.19 closed live themetracbot
#32308 THEME: Handcraft Expo – 1.0.20 closed live themetracbot
#32326 THEME: Handcraft Expo – 1.0.21 closed live themetracbot
#32587 THEME: Handcraft Expo – 1.0.22 closed live themetracbot
#32653 THEME: Handcraft Expo – 1.0.23 closed live themetracbot
#33762 THEME: Handcraft Expo – 1.0.24 closed live themetracbot
#33826 THEME: Handcraft Expo – 1.0.25 closed live themetracbot
#35299 THEME: Handcraft Expo – 1.0.26 closed live themetracbot
#36675 THEME: Handcraft Expo – 1.0.27 closed live themetracbot
#37855 THEME: Handcraft Expo – 1.0.28 closed live themetracbot
#39438 THEME: Handcraft Expo – 1.0.29 closed live themetracbot
#39520 THEME: Handcraft Expo – 1.0.30 closed live themetracbot
#40303 THEME: Handcraft Expo – 1.1.0 closed live themetracbot
#40411 THEME: Handcraft Expo – 1.1.1 closed live themetracbot
#42213 THEME: Handcraft Expo – 1.1.2 closed live themetracbot


https://themes.svn.wordpress.org/handcraft-expo/1.0.11/screenshot.png

#45 @_Y_Power
2 years ago

Hey @poena ,
here I come with the new fixes.
Thank you again for all your patience, dedication and great tips!

Translations
Translated all the parts you pointed out.
It's amazing how many times these un-translateble bits escaped my scrutiny... does it say something about my ability to check...?! ;)

Navigation php error
Should have fixed it by changing the condition to '!= 0': worked fine on a new WordPress install and I assume shouldn't cause any problem but I couldn't do a debug check.

Comments
Removed the variables which were indeed not used.

Pocket Menu
Menu size should now be tall enough even with no social links: if possible, I'd like to leave the menu opened by default as a reminder of the 'important' notice in the readme about selecting menus before customizing since I didn't style menu items for the same reason.
Should I add another notice somewhere?

wp_link_pages
Finally understood what you were trying to tell me: I envy your patience :).
Moved all calls in the above div: I didn't change styles for the moment since it seemed ok to me.

Templates styles
Fantastic tip about filtering body_class: completely missed it!
Added filter in functions.php, added styles in styles.css and removed css files.
Removed the opening-closing 'style' tags: source looks way prettier now ;).

handcraft_expo_customize_css_output
Moved the transitions to the style sheet but left the opacity in since I'd like the 'none' option to have no effects on hover.
Tried also to re-arrange a bit the code but I couldn't really come up with any game-changer...

handcraft_expo_GetPreviewerPost
Links are tempting to click, agreed!
Removed all links by 'strip_tags' on get_the_content and meta but left title colors and style: how about that? :)


Updated '.pot' and translated '.po' files from it.

Also fixed a few things and added a 'Sidebar toggle button' in the 'Menu Sidebar' section of the customizer.

Last edited 2 years ago by _Y_Power (previous) (diff)

#46 @poena
2 years ago

Hi, I need some more time to check your update, hopefully the next few days.

#47 @themetracbot
2 years ago

  • Summary changed from THEME: Handcraft Expo – 1.0.11 to THEME: Handcraft Expo – 1.0.12

Handcraft Expo - 1.0.12

A WordPress Theme focused on graphical impact and simplicity: a traditional left sidebar spin-off helps you &#8216;streamline what really matters&#8217;. Geared towards personal or small-sized handcrafting businesses use. GNU GPL v3 guarantees all user&#8217;s freedoms. French and Italian translations included.

Theme URL - http://handcraftexpo.nouveausiteweb.fr
Author URL - http://ypower.nouveausiteweb.fr

SVN - https://themes.svn.wordpress.org/handcraft-expo/1.0.12
ZIP - https://wordpress.org/themes/download/handcraft-expo.1.0.12.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=handcraft-expo/1.0.11&new_path=handcraft-expo/1.0.12

History:

Ticket Summary Status Resolution Owner
#27664 THEME: Handcraft Expo – 1.0.14 closed live poena

(this ticket)

#31427 THEME: Handcraft Expo – 1.0.15 closed live themetracbot
#31505 THEME: Handcraft Expo – 1.0.16 closed live themetracbot
#31811 THEME: Handcraft Expo – 1.0.17 closed live themetracbot
#32206 THEME: Handcraft Expo – 1.0.18 closed live themetracbot
#32212 THEME: Handcraft Expo – 1.0.19 closed live themetracbot
#32308 THEME: Handcraft Expo – 1.0.20 closed live themetracbot
#32326 THEME: Handcraft Expo – 1.0.21 closed live themetracbot
#32587 THEME: Handcraft Expo – 1.0.22 closed live themetracbot
#32653 THEME: Handcraft Expo – 1.0.23 closed live themetracbot
#33762 THEME: Handcraft Expo – 1.0.24 closed live themetracbot
#33826 THEME: Handcraft Expo – 1.0.25 closed live themetracbot
#35299 THEME: Handcraft Expo – 1.0.26 closed live themetracbot
#36675 THEME: Handcraft Expo – 1.0.27 closed live themetracbot
#37855 THEME: Handcraft Expo – 1.0.28 closed live themetracbot
#39438 THEME: Handcraft Expo – 1.0.29 closed live themetracbot
#39520 THEME: Handcraft Expo – 1.0.30 closed live themetracbot
#40303 THEME: Handcraft Expo – 1.1.0 closed live themetracbot
#40411 THEME: Handcraft Expo – 1.1.1 closed live themetracbot
#42213 THEME: Handcraft Expo – 1.1.2 closed live themetracbot


https://themes.svn.wordpress.org/handcraft-expo/1.0.12/screenshot.png

#48 @_Y_Power
2 years ago

Hi @poena ,
no worries, I understand: take your time.
I'll then ask you to check this latest update, which fixes some mistakes I've been doing on the previous release.
Thank you again for all your efforts.

#49 @poena
2 years ago

  • Cc emiluzelac added

Hi
Here is a thought, What would happen if you deleted mobile-html.php and styled the responsive pages with the conditionals and the css?

I am not sure why you are renaming the css classes when you could be using media queries.
I compared the part for "is_archive" in mobile-html.php with archive.php
and they are not that different, but for example:
archive.php uses

<p class="handcraftExpo-page-default-content"><?php the_excerpt(); ?></p>

and mobile-html.php uses

			<div class="mobile-content-archive">
				<p><div <?php post_class(); ?>><?php the_excerpt(); ?></div>								
<?php edit_post_link( __('Edit', 'handcraft-expo'), '<p>', '</p>'); ?>
			</div>

here, the closing </p> is missing, and the class mobile-content-archive is not used anywhere else.


I am still having problems with the page navigation. Now I see the offset error no matter how many pages there are. The problem is not with printing the navigation, but on the lines above it.
This is beyond my scope. @emiluzelac can you help us check the page navigation in https://themes.svn.wordpress.org/handcraft-expo/1.0.12/page.php?

Notice: Undefined offset: -1 in themes\handcraft-expo\page.php on line 64

$handcraft_expo_prevID = $handcraft_expo_pages[$handcraft_expo_current-1];

Did you intend for the menu to be stickied? When I scroll, the adminbar is outside the viewport but the menu remains visible, see gif.


Language
Missing translations:

custom-tags.php: "Tags:"

the_tags( 'Tags: ', ' || ' );

mobile-html.php
line 5 "Menu"
line 88 "continue..."

Last edited 2 years ago by poena (previous) (diff)

@poena
2 years ago

#50 @emiluzelac
2 years ago

I can't reproduce notice issue.

#51 @_Y_Power
2 years ago

Hey @poena , hi @emiluzelac !

I've fixed the issues you pointed out, except:

Pages navigation
I haven't been able to reproduce the error but still couldn't test it on a fresh install as you suggested: I'll try it asap but I can't see the error on all pages... I'll be holding the upload to give you time to address me about it.

mobile-html.php
I completely understand your point on this file and I could certainly do it the way you suggested and it would totally make sense: there are however a few reasons why I chose to do it this way and I'd like to know if I really 'need' to change it since it would complicate things a bit for me...
The main reasons I did it:

  • to separate the two structures for easier 'pocket' layout customization (theme's website will feature simple howtos and templates);
  • to inhibit the 'reply to comment' by repeating the comment id.

I also like it better this way for other personal preferences but I'll obviously change it if that's unacceptable and/or causes troubles.

As usual, thank you so much for all your help! :)

#52 @poena
2 years ago

No it is not required :)

I recommend you to fix the html errors.

#53 @poena
2 years ago

  • Cc emiluzelac removed

You can keep the page navigation like it is, if it turns out that users are having this problem, it can be fixed in an update.

#54 @themetracbot
2 years ago

  • Summary changed from THEME: Handcraft Expo – 1.0.12 to THEME: Handcraft Expo – 1.0.13

Handcraft Expo - 1.0.13

A WordPress Theme focused on graphical impact and simplicity: a traditional left sidebar spin-off helps you &#8216;streamline what really matters&#8217;. Geared towards personal or small-sized handcrafting businesses use. GNU GPL v3 guarantees all user&#8217;s freedoms. French and Italian translations included.

Theme URL - http://handcraftexpo.nouveausiteweb.fr
Author URL - http://ypower.nouveausiteweb.fr

SVN - https://themes.svn.wordpress.org/handcraft-expo/1.0.13
ZIP - https://wordpress.org/themes/download/handcraft-expo.1.0.13.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=handcraft-expo/1.0.12&new_path=handcraft-expo/1.0.13

History:

Ticket Summary Status Resolution Owner
#27664 THEME: Handcraft Expo – 1.0.14 closed live poena

(this ticket)

#31427 THEME: Handcraft Expo – 1.0.15 closed live themetracbot
#31505 THEME: Handcraft Expo – 1.0.16 closed live themetracbot
#31811 THEME: Handcraft Expo – 1.0.17 closed live themetracbot
#32206 THEME: Handcraft Expo – 1.0.18 closed live themetracbot
#32212 THEME: Handcraft Expo – 1.0.19 closed live themetracbot
#32308 THEME: Handcraft Expo – 1.0.20 closed live themetracbot
#32326 THEME: Handcraft Expo – 1.0.21 closed live themetracbot
#32587 THEME: Handcraft Expo – 1.0.22 closed live themetracbot
#32653 THEME: Handcraft Expo – 1.0.23 closed live themetracbot
#33762 THEME: Handcraft Expo – 1.0.24 closed live themetracbot
#33826 THEME: Handcraft Expo – 1.0.25 closed live themetracbot
#35299 THEME: Handcraft Expo – 1.0.26 closed live themetracbot
#36675 THEME: Handcraft Expo – 1.0.27 closed live themetracbot
#37855 THEME: Handcraft Expo – 1.0.28 closed live themetracbot
#39438 THEME: Handcraft Expo – 1.0.29 closed live themetracbot
#39520 THEME: Handcraft Expo – 1.0.30 closed live themetracbot
#40303 THEME: Handcraft Expo – 1.1.0 closed live themetracbot
#40411 THEME: Handcraft Expo – 1.1.1 closed live themetracbot
#42213 THEME: Handcraft Expo – 1.1.2 closed live themetracbot


https://themes.svn.wordpress.org/handcraft-expo/1.0.13/screenshot.png

#55 @_Y_Power
2 years ago

Hi @poena ,
here I come again... :)

Fixes in this update:

HTML errors
As usual, you were right: source was pretty full of errors of different nature. Made some changes and last checks pretty much resulted in errors only concerning duplicate ids, which are expected. Let me know if I should make further changes.

Pocket Menu
'Sticky' admin menu should be fixed now.

Pages Navigation
Still didn't have the chance to test it on a new debug enabled install (...).
I did however checked on other installs and found no errors: I'll check and fix asap.

Translations
Fixed strings you pointed out and updated sources and .po files.


You might also want to check the new main sidebar menu toggle switch mechanism/script and the new 'Custom CSS' customizer sections (did I do it properly?): I know I change things all the times, please don't scream at me... :)

Again, Thank You.

Last edited 2 years ago by _Y_Power (previous) (diff)

#56 @poena
2 years ago

Hi
In functions.php, the sanitize_callback for custom css needs to be wp_filter_nohtml_kses() or wp_strip_all_tags().

		$wp_customize->add_setting('handcraft-expo_custom_css', array(
			'default' => '',
			'sanitize_callback' => 'esc_textarea',
			'transport' => 'refresh',
		));

#57 @themetracbot
2 years ago

  • Summary changed from THEME: Handcraft Expo – 1.0.13 to THEME: Handcraft Expo – 1.0.14

Handcraft Expo - 1.0.14

A WordPress Theme focused on graphical impact and simplicity: a traditional left sidebar spin-off helps you &#8216;streamline what really matters&#8217;. Geared towards personal or small-sized handcrafting businesses use. GNU GPL v3 guarantees all user&#8217;s freedoms. French and Italian translations included.

Theme URL - http://handcraftexpo.nouveausiteweb.fr
Author URL - http://ypower.nouveausiteweb.fr

SVN - https://themes.svn.wordpress.org/handcraft-expo/1.0.14
ZIP - https://wordpress.org/themes/download/handcraft-expo.1.0.14.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=handcraft-expo/1.0.13&new_path=handcraft-expo/1.0.14

History:

Ticket Summary Status Resolution Owner
#27664 THEME: Handcraft Expo – 1.0.14 closed live poena

(this ticket)

#31427 THEME: Handcraft Expo – 1.0.15 closed live themetracbot
#31505 THEME: Handcraft Expo – 1.0.16 closed live themetracbot
#31811 THEME: Handcraft Expo – 1.0.17 closed live themetracbot
#32206 THEME: Handcraft Expo – 1.0.18 closed live themetracbot
#32212 THEME: Handcraft Expo – 1.0.19 closed live themetracbot
#32308 THEME: Handcraft Expo – 1.0.20 closed live themetracbot
#32326 THEME: Handcraft Expo – 1.0.21 closed live themetracbot
#32587 THEME: Handcraft Expo – 1.0.22 closed live themetracbot
#32653 THEME: Handcraft Expo – 1.0.23 closed live themetracbot
#33762 THEME: Handcraft Expo – 1.0.24 closed live themetracbot
#33826 THEME: Handcraft Expo – 1.0.25 closed live themetracbot
#35299 THEME: Handcraft Expo – 1.0.26 closed live themetracbot
#36675 THEME: Handcraft Expo – 1.0.27 closed live themetracbot
#37855 THEME: Handcraft Expo – 1.0.28 closed live themetracbot
#39438 THEME: Handcraft Expo – 1.0.29 closed live themetracbot
#39520 THEME: Handcraft Expo – 1.0.30 closed live themetracbot
#40303 THEME: Handcraft Expo – 1.1.0 closed live themetracbot
#40411 THEME: Handcraft Expo – 1.1.1 closed live themetracbot
#42213 THEME: Handcraft Expo – 1.1.2 closed live themetracbot


https://themes.svn.wordpress.org/handcraft-expo/1.0.14/screenshot.png

#58 @_Y_Power
2 years ago

Hi @poena ,

changed the sanitize_callback to wp_filter_nohtml_kses and fixed the new sidebar menu toggle 'scrollTo' animation.
Thank you!

#59 @poena
2 years ago

  • Status changed from reviewing to approved

The sanitizing setting has been fixed.
I will mark the theme as approved, and an admin will look at it before it goes live.
The current wait time for an admin review is about 3 weeks.

#60 @_Y_Power
2 years ago

Hey @poena ,
that's great news! :)
I then kindly ask you two things:

1) could you wait the next version to approve it (I'd like to make the code more readable as we previously discussed) and

2) can I add your name to the credits? (it will be something 'Thank you to Carolina and all the WordPress Theme Review Team for helping me make this theme a WAY better theme') :).

Thank you again

#61 @karmatosed
2 years ago

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

Congratulations, this theme is now live!

It may take a little while for your theme to show up in the directory, but it will.

#62 @_Y_Power
2 years ago

@poena
I really want to thank you for all the help and the dedication: you rock!

@karmatosed
Great news, I'm very happy! :)

Note: See TracTickets for help on using tickets.