This is a very simple paper-on-wood theme for D7. It's based on photo of a piece of stationary on my own desk. The "paper" will match the length of your content and has a "torn" edge at the bottom. It's a fixed width theme with sidebars, blocks above and below the content, both on and off the "page." Features include:
- Suckerfish drop-down menu support
- Webfont support through the theme settings form (see readme)

This theme is for Drupal 7. Letter is a Zen sub-theme; you must install Zen 3.x for Letter to work.

This theme was created for a non-profit, the American Civil War Association. But I thought other people might like it too, so I'm offering it on Drupal.org.

Project page: http://drupal.org/sandbox/koppie/1458942
Git: git clone http://git.drupal.org/sandbox/koppie/1458942.git letter
Demonstration of the theme here: http://acwa.org

Reviews of other projects:
http://drupal.org/node/1798522#comment-6877714
http://drupal.org/node/1781984#comment-6877820
http://drupal.org/node/1512124#comment-6877864

CommentFileSizeAuthor
screenshot.png133.68 KBkoppie
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

patrickd’s picture

patrickd CreditAttribution: patrickd commented
Status: Needs review » Needs work

welcome,

It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.
(This has to be fixed before you can switch back to "needs review")

while waiting for an in-depht review of your theme you can start out fixing some coding style issues detected by automated tools:
http://ventral.org/pareview/httpgitdrupalorgsandboxkoppie1458942git

You can also get a review bonus and we will come back to your application sooner.

koppie’s picture

koppie CreditAttribution: koppie commented
Status: Needs work » Needs review

@patrickd: Thank you for pointing me in the right direction. As you can tell I'm pretty new at this. I've fixed the following issues:

  • working from head
  • files left over in the master branch
  • coding problems left over from my text editor (it was leaving hidden draft versions in the directory)
  • coding problems in an unused template file (zen subtheme)

The style sniffer still detects errors in the CSS, this is also a product of the zen sub-theme. I don't know what kind of discussion has been had regarding non-standard CSS in the zen theme (eg. uppercase hex color values) but if it's allowed in that project, it ought to be allowed in mine. Zen is listed as a dependency of my project since this is a sub-theme. If this is considered unacceptable for new projects then I will update my code without further argument.

Thanks again for your help.

--Jordan

tonybuckingham’s picture

tonybuckingham CreditAttribution: tonybuckingham commented
Status: Needs review » Needs work

A "diff" between the STARTERKIT directory included with the "Zen" theme and this theme reveals that only a few files were modified. The main innovation appears to be in page-backgrounds.css:

body {
  background: url('../images/table.jpg');
}
#main-wrapper {
  background: url('../images/paper-top.png') no-repeat;
}
#content {
    background: url('../images/paper-middle.png') repeat-y;
}
.region-footer {
  color: white;
  background: url('../images/paper-bottom.png') no-repeat;
  padding-top: 20px;
}

Problem is that of the four images referred to in the CSS, only "paper-bottom.png" exists in the "images" directory.

Line 10, theme-settings.php:

function STARTERKIT_form_system_theme_settings_alter(&$form, &$form_state)  {

. . . still uses the STARTERKIT prefix. If you're not going to use the theme_settings.php you can just delete this file.

Not sure this is really a theme, considering the only modifications were made to CSS, but I'm not sure what the rules are about what can and can't be considered a useful "theme" for inclusion in Drupal's repository.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Status: Needs work » Closed (won't fix)

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

koppie’s picture

koppie CreditAttribution: koppie commented
Status: Closed (won't fix) » Needs review

Thank you Tony for your feedback. Back in March I was very inexperienced and I appreciate you pointing me in the right direction. I've solved all the problems and problems reported by ventral, including:

  • restored graphics for backgrounds
  • removed unused code from template.php
  • removed all references to STARTERKIT and removed files and directories I wasn't using
  • removed master branch
  • removed .project files
  • cleaned up text in readme.txt
  • cleaned up code in theme-settings.php
  • Added doxygen-compliant comments to template.php and theme-settings.php (doxygen is rather picky, isn't it?)

In addition I made some improvements:

  • improved layout, structure & colors
  • added support for sidebars
  • added built-in webfont support through the theme settings form, via either javascript or css (different webfont providers use different methods)

I know this is a simple theme, but that was intentional. The textures were handmade using my own photographs. I think it's a lovely effect and wanted to share with the community; there aren't a lot of themes that use wood & paper. I don't know if there is a stated criteria for how fancy a theme must be in order to be accepted on D.org, but there are some pretty simple themes that have been accepted in the past.

I will also contribute reviews to the issue queue so this project will get reviewed faster.

I look forward to your feedback. :-)

bryanbraun’s picture

bryanbraun CreditAttribution: bryanbraun commented

I just reviewed your theme. A lot of it looked fine, but I did find a few things:

  • I ran your code through the PAReview tool and there were some things flagged. See http://ventral.org/pareview/httpgitdrupalorgsandboxkoppie1458942git. It looks like a lot of them were CSS formatting issues. Check out theDrupal CSS Coding Standards, for reference.
  • For what it's worth, a few design elements seemed unfinished.
    • There is no right-padding on body paragraphs, causing the text to sit up against the right side of the "page"
    • The wood background image has a white blur in the top left corner, which creates a distraction when the image repeats. I think trimming it off the top of the image would make a big difference.
  • I don't think this is would hold up the project, but it would be nice to have documentation or examples written somewhere on how to use the custom webfonts settings you provided.

I appreciate that you've cleaned up a lot of things and added customizations since the last review. From what I can tell from template.php and theme-settings.php, you are using the Drupal functions and APIs properly. Keep up the good work.

Also, FYI, your git URL in the issue summary doesn't work for other people, since it has your username in it. The one you want to post is git clone http://git.drupal.org/sandbox/koppie/1458942.git letter

bryanbraun’s picture

bryanbraun CreditAttribution: bryanbraun commented
Status: Needs review » Needs work

Changing status...

bryanbraun’s picture

bryanbraun CreditAttribution: bryanbraun commented
Issue summary: View changes

Added support for sidebars and pointed to demonstration site

koppie’s picture

koppie CreditAttribution: koppie commented
Status: Needs work » Needs review

Bryan thanks for your feedback! I've made the following fixes:

  • fixed all Ventral problems, removed or cleaned up a lot of css code
  • fixed paragraph padding for all sidebar combinations
  • fixed background; it now repeats seamlessly in both directions
  • added documentation to the readme file on how to use the webfonts feature. Also added a built-in webfont for a demonstration (hosted by Google Webfonts)
  • fixed git link in issue summary

I'll be honest: this process has made me a better Drupal developer. Thank you everyone for your help.

--Jordan

cackle’s picture

cackle CreditAttribution: cackle commented

Hello,

fixed all Ventral problems, removed or cleaned up a lot of css code

That's ok.
Git link is also ok.
And what about cross-browser issues? Have you test it on different browsers?
I also would recommend you to make a sprite of images and to minify css.

koppie’s picture

koppie CreditAttribution: koppie commented

Cackle thanks for the review. I can't minify the CSS because it wouldn't pass Ventral testing; see http://ventral.org/pareview and http://drupal.org/node/894256. Besides, I don't think it's necessary if you turn on CSS aggregation and compression. I also don't think any of my graphics would benefit by being turned into sprites.

I have, however, done full cross-browser compatibility testing. The site looks and works fine in IE8, IE9, IE10, Firefox, Chrome and Safari.

Is there anything else standing in the way of RTBC?

koppie’s picture

koppie CreditAttribution: koppie commented
Issue summary: View changes

fixed git command

koppie’s picture

koppie CreditAttribution: koppie commented
Issue tags: +PAreview: review bonus

I've completed the review bonus program. Lay some love on me. :-)

monymirza’s picture

monymirza
Primary language Urdu
Location Islamabad
CreditAttribution: monymirza commented
Status: Needs review » Needs work

You need some cross browser work. specially in IE7-IE8. in demo link, bottom regions are collapsed.

is it compatible with Zen 5.x ???

koppie’s picture

koppie CreditAttribution: koppie commented
Status: Needs work » Needs review

Looks fine in my test of IE8 on Win7. Please supply a screenshot to show me the problem you're seeing in IE8 or give steps to reproduce the problem.

I will not support IE7; please see http://theie7countdown.com/ . IE7 support is not required for projects to be approved on D.org.

This theme is not compatible with Zen 5.x; it requires Zen 3.x. This requirement is listed in the issue description and also in the readme file. Zen 3.1 is still available for download on the Zen project page.

monymirza’s picture

monymirza
Primary language Urdu
Location Islamabad
CreditAttribution: monymirza commented

while theme is looks good. but i suggest to theme search block/page and user pages...

also put a demo page containing typography,,,

koppie’s picture

koppie CreditAttribution: koppie commented

There is a page on the demo site talking about webfonts: http://acwa.org/about-site

@MonyMirza I appreciate that you've given multiple reviews of my project. However, before you do another, please review http://drupal.org/node/894256. In particular:

Even if the code is perfect, you can still offer your experience as a Drupal developer. This application is not just about making sure code is perfect, its about ensuring that we have quality contributors to Drupal. If the code is good enough to accept the application, mark as reviewed, but make suggestions on how the applicant can improve their code to make it better for the community. For instance, suggesting Views integration, looking at X module, or saying how adding a hook here would make this much more extensible, etc.

If there's something standing in the way of this project getting approved, please let me know and I'll fix it. Otherwise, can I get RTBC please?

monymirza’s picture

monymirza
Primary language Urdu
Location Islamabad
CreditAttribution: monymirza commented
Status: Needs review » Reviewed & tested by the community

Hi,

i have test theme. Overall, it is good. Hopefully you make improvements in next release.

one suggestion,
> theme is good with having only Logo. make some improvements if logo and slogan also placed.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Status: Reviewed & tested by the community » Fixed

Review of the 7.x-1.x branch:

  • Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
    FILE: /home/klausi/pareview_temp/theme-settings.php
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 10 | WARNING | Hook implementations should not duplicate @param documentation
--------------------------------------------------------------------------------

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

Thanks for your contribution, koppie!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

koppie’s picture

koppie CreditAttribution: koppie commented
Issue tags: -PAreview: review bonus

Thank you @monymirza and @klausi! I will make the changes before I publish the theme.

I've been having trouble with doxygen; it's very demanding and there aren't enough examples in the documentation. It expects @param documentation but now PAReview is complaining about it? I don't think it's a syntax error. Are there rules about which function parameters are supposed to be documented and which aren't?

Thanks!!

(I'm also reviewing the review bonus tag because it's not necessary any more.)

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) CreditAttribution: commented
Issue summary: View changes

completed review bonus program