WordPress.org

Make WordPress Themes

Opened 4 years ago

Closed 4 years ago

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

THEME: Ridizain - 1.0.28

Reported by: ZGani Owned by: Frank Klein
Priority: theme update Keywords: theme-ridizain
Cc: zulfikarnore@…, chipbennett

Description

Ridizain - 1.0.28

Ridizain is a redesigned version of the default theme Twenty Fourteen - Create a responsive magazine website with a sleek, modern design - full width and fully centred. Feature your favourite homepage content in either a grid or a slider. Use the three widget areas + the custom recent posts and the ephemera widgets to customize your website, and change your content's layout with a full-width page template and a contributor page to show off your authors. With full color control for many of the theme's elements, creating a magazine website with WordPress has never been easier. View demo at: http://www.wpstrapcode.com/ridizain/

Theme URL - http://wordpress.org/themes/ridizain
Author URL - http://ridizain.com/

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=/ridizain/1.0.27&new_path=/ridizain/1.0.28

History:

Ticket Summary Status Resolution Owner
#15730 THEME: Ridizain - 1.0.01 closed closed-newer-version-uploaded Themonic
#15873 THEME: Ridizain - 1.0.02 closed live rohitink
#15989 THEME: Ridizain - 1.0.03 closed live tskk
#16085 THEME: Ridizain - 1.0.05 closed live tskk
#16195 THEME: Ridizain - 1.0.07 closed live rohitink
#16292 THEME: Ridizain - 1.0.08 closed live ZGani
#16408 THEME: Ridizain - 1.0.09 closed live tskk
#16540 THEME: Ridizain - 1.0.10 closed live tskk
#16600 THEME: Ridizain - 1.0.11 closed live rohitink
#16620 THEME: Ridizain - 1.0.12 closed live robin90
#16699 THEME: Ridizain - 1.0.13 closed live tskk
#16711 THEME: Ridizain - 1.0.14 closed live robin90
#16770 THEME: Ridizain - 1.0.16 closed live jcastaneda
#16811 THEME: Ridizain - 1.0.17 closed live emiluzelac
#16897 THEME: Ridizain - 1.0.18 closed live catchthemes
#17004 THEME: Ridizain - 1.0.19 closed live catchthemes
#17119 THEME: Ridizain - 1.0.20 closed live tskk
#17256 THEME: Ridizain - 1.0.24 closed live robin90
#17322 THEME: Ridizain - 1.0.25 closed live robin90
#17487 THEME: Ridizain - 1.0.26 closed live tskk
#17630 THEME: Ridizain - 1.0.27 closed live tskk
#17746 THEME: Ridizain - 1.0.28 closed closed-newer-version-uploaded Frank Klein

(this ticket)

#17757 THEME: Ridizain - 1.0.30 closed live catchthemes
#17927 THEME: Ridizain - 1.0.31 closed live catchthemes
#18081 THEME: Ridizain - 1.0.32 closed live catchthemes
#18094 THEME: Ridizain - 1.0.33 closed live tskk
#22736 THEME: Ridizain - 1.0.35 closed live karmatosed
#24442 THEME: Ridizain – 1.0.37 closed live jcastaneda


https://themes.svn.wordpress.org/ridizain/1.0.28/screenshot.png

Change History (8)

#1 @Frank Klein
4 years ago

  • Owner set to Frank Klein
  • Status changed from new to reviewing

#2 @Frank Klein
4 years ago

  • Resolution set to not-approved
  • Status changed from reviewing to closed

Blockers:

  • $wp_customize->add_setting( 'ridizain_featured_order' ) needs a callback function for sanitization.
  • ridizain_CleanupCSS($buffer): Minifying CSS is plugin territory.
  • ridizain_HookCSS(): This should be done with wp_add_inline_style().

Recommended:

  • add_action( 'init', 'ridizain_remove_pre_get_posts', 31 ): It's not recommended for themes to hook into init, as it is already a major bottleneck. I'd be easer to only conditionally add these actions based on the option.
  • ridizain_remove_featured_sections(): Instead of removing these, why not conditionally add these actions based on the option.

#3 @ZGani
4 years ago

  • Cc chipbennett added

Hello Frank,

Thank you for the review but given this is an already approved theme in the repository, I think this ticket was closed rather abruptly - at the very least you should have pointed out the issues and given me the opportunity to address them first!

ridizain_CleanupCSS($buffer) only removes white spaces and carriage returns from the in head printed out styles - is that classed as minification? I mean the style is still readable!

Isn't wp_add_inline_style() for template inline styles rather than styles printed out in the header which are global?

@chipbennett - May we please have this ticket reopened so that we can get the raised issues resolved? I believe it was closed rather abruptly.

If you could also clarify on my last two points it would help me understand better.

Thanks, Zulf

#4 @chipbennett
4 years ago

  • Resolution not-approved deleted
  • Status changed from closed to reopened

#5 @chipbennett
4 years ago

  • Status changed from reopened to reviewing

@Frank Klein thank you for the review. Please note that our workflow has changed somewhat. The standard practice is to keep tickets open to allow the developer time to respond to issues and to submit revisions - especially for Theme Updates (currently approved Themes).

While in general minimization is Plugin territory, in this case, the function in question is merely removing whitespace from Theme option-generated CSS. I think that's acceptable. (Also: as this is directly output CSS, it would be missed by most/all minification scripts, which generally target linked stylesheets/scripts.)

While wp_add_inline_style() is a useful function (and one that I should probably look at making use of myself), I don't think it's necessarily appropriate/required here. The CSS being added is custom CSS that may or may not be related to/dependent upon a Theme-enqueued stylesheet. Hooking directly into wp_head() is fine here, I think.

#6 @ZGani
4 years ago

Thank you for the clarification @chipbennett

@Frank - uploaded a revised version with the required sanitization but tract seems to have created a new ticket here: https://themes.trac.wordpress.org/ticket/17757

Thanks,
Zulf

Last edited 4 years ago by ZGani (previous) (diff)

#7 follow-up: @Frank Klein
4 years ago

@chipbennett thanks for the explanations on the new workflow. I'll keep this in mind for the future.

As far as the two blockers are concerned, it's clumsy and inefficient, but I agree with you that nothing in the guidelines currently prevents this. I'll make note of this for future reviews.

Seeing that the update has been approved, can we safely close this ticket?

#8 in reply to: ↑ 7 @chipbennett
4 years ago

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

Replying to Frank Klein:

@chipbennett thanks for the explanations on the new workflow. I'll keep this in mind for the future.

As far as the two blockers are concerned, it's clumsy and inefficient, but I agree with you that nothing in the guidelines currently prevents this. I'll make note of this for future reviews.

By all means, point these kinds of things out to developers as recommendations. Theme developers comprise a broad spectrum of PHP/development skill and experience, and this is a great opportunity for us to learn from each other. (Like with wp_add_inline_style(), which I keep forgetting to look into implementing.) But because of the broad spectrum of skill/experience, we try to tread lightly on requiring certain coding styles/practices.

Seeing that the update has been approved, can we safely close this ticket?

I'll close this one as newer-version-uploaded.

Note: See TracTickets for help on using tickets.