[theme-reviewers] Fwd: THEME: Rachel - 0.8 (closed theme: not-accepted)
Tom Lany
mail at tomlany.net
Sun Jun 27 21:36:15 UTC 2010
Looked at the latest version of this (Rachel 0.93), and I think it looks
fine to approve, and it addresses Edward's comments yesterday. Can't
write on a ticket because Trac isn't creating new tickets today for some
reason.
Tom Lany
http://tomlany.net
On 6/27/10 11:31 AM, // ravi wrote:
> On Jun 27, 2010, at 9:59 AM, chip at chipbennett.net wrote:
>
>> Keep in mind that themes hosted in the theme repository must include
>> support for the wp_head() and wp_footer() hooks - so it is definitely
>> important that the theme layout and design account for content that gets
>> added using those hooks.
>>
>>
> Sure, of course. The support is already there. The layout/design is the issue currently, but I think there are workarounds.
>
>
>
>> I would *strongly* advise against using a table-based layout. Absolute
>> positioning of DIVs is fine. Perhaps you can look into adding an
>> absolutely positioned footer DIV.
>>
>> At this point, though, your questions (regarding HTML/CSS implementation)
>> are probably best-suited to be asked in the wordpress.org support forums.
>>
>
> No worries. My ramblings on implementation (quoted below) were intended to give Cais an idea of my thought process, not to pose questions about them. In fact, I like his suggestion of throwing the footer into the right container DIV, which I think works better than other alternatives; and I have already implemented it. I will drop the Cc:theme-reviewers for future messages.
>
> Regards,
>
> —ravi
>
>
>
>
>> Regards,
>>
>>
>>> On Jun 26, 2010, at 10:29 PM, Edward Caissie wrote:
>>>
>>>> A quick fix is to put the wp_footer() call inside the "container" div.
>>>> It's currently outside.
>>>>
>>>> Just a suggestion …
>>>>
>>>>
>>> Hello Cais,
>>>
>>> Hmm, visually that might not be what I want. As you can probably tell, the
>>> problem arises from the fact that I have absolute positioning for the two
>>> DIVs (one for the sidebar, which I fondly call the “stripe” ;-)) and one
>>> for the post(s) (+/-comments), which I call “container”.
>>>
>>> As you suggest, putting the footer inside the container, will position it
>>> at the bottom, but include it within the container. Which is really
>>> intended (visually) to hold only posts (in the index/home page) or
>>> page/post+comments (in single mode).
>>>
>>> I could remove the absolute positioning and float both DIVs left, but then
>>> I will have to specify a width for both… which is a bit of a philosophical
>>> problem for a web old-timer like me ;-), who favours flexible over fixed
>>> widths. And if I go the ‘float’ route, even if I do specify a decent width
>>> for the container that is not too small or too wide, it might wrap to the
>>> bottom depending on user browser sizing. I could play games with wrapping
>>> the footer in an absolutely positioned DIV with bottom: 0, but I think I
>>> would still need to make a guess on the height of this wrapper/footer, to
>>> make room for it below the container.
>>>
>>> The 100% fix of course is using TABLEs. But that is so unfashionable these
>>> days that I hesitate to go that route.
>>>
>>> Thank you again to both of you for all the productive comments. I will
>>> either bite the bullet and go with the floated DIVs, or use your
>>> suggestion of throwing the footer into the container. I am leaning towards
>>> the latter.
>>>
>>> —ravi
>>>
>>>
>>>
>>>
>>>> Cais.
>>>>
>>>> On Sat, Jun 26, 2010 at 10:09 PM, // ravi<ravi at g8o.net> wrote:
>>>> On Jun 26, 2010, at 10:07 PM, Tom Lany wrote:
>>>>
>>>>> Comment from Cais (another theme reviewer/admin):
>>>>>
>>>>> I really like the theme but you might look at how it handles items
>>>>>
>>>> hooked into the footer.
>>>>
>>>>> I use one of my simpler plugins (BNS Login) that hooks into the footer
>>>>>
>>>> and find it firmly positioned at the top of the page!?
>>>>
>>>>> I would expect the footer to be at the bottom of the page ...
>>>>>
>>>>> Cais.
>>>>>
>>>>> If you could take a look at this, that would be great.
>>>>>
>>>>>
>>>> Tom, will do. Installing the plugin right now, on my development blog.
>>>>
>>>> —ravi
>>>>
>>>>
>>>>
>>>>> Tom Lany
>>>>> http://tomlany.net
>>>>>
>>>>>
>>>>> On 6/26/10 9:01 PM, // ravi wrote:
>>>>>
>>>>>> On Jun 26, 2010, at 9:16 PM, Tom Lany wrote:
>>>>>>
>>>>>>
>>>>>>> I am recommending that an admin look at this and that it be
>>>>>>>
>>>> approved. See feedback: http://themes.trac.wordpress.org/ticket/176.
>>>> Great work!
>>>>
>>>>>>>
>>>>>>>
>>>>>> Tom,
>>>>>>
>>>>>> thank you and I must once again say that your feedback was excellent!
>>>>>>
>>>>>> —ravi
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>> Tom Lany
>>>>>>> http://tomlany.net
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 6/26/10 6:03 PM, // ravi wrote:
>>>>>>>
>>>>>>>
>>>>>>>> On Jun 26, 2010, at 6:27 PM, Tom Lany wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>> Thanks for the note. Feedback ha been posted:
>>>>>>>>>
>>>> http://themes.trac.wordpress.org/ticket/173
>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>> Ah, good catch on the function_exists() naked literal. That’s fixed
>>>>>>>>
>>>> as well as a few other similar ones and also the content tag.
>>>> Uploaded 0.92. Sorry for the confusion regarding 0.95. 0.91 was the
>>>> previous number and this one is 0.92.
>>>>
>>>>>>>> thank you,
>>>>>>>>
>>>>>>>> —ravi
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>> Tom Lany
>>>>>>>>> http://tomlany.net
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 6/26/10 12:34 PM, // ravi wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> On Jun 25, 2010, at 5:56 PM, Tom Lany wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> Thanks for fixing these issues. The theme looks a lot better.
>>>>>>>>>>>
>>>> Take a look here: http://themes.trac.wordpress.org/ticket/166
>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>> Hello Tom,
>>>>>>>>>>
>>>>>>>>>> Fixed all of the above (I hope!), added license.txt and
>>>>>>>>>>
>>>> background colour/image support. Uploaded 0.95. Have at it! ;-)
>>>>
>>>>>>>>>> —ravi
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> Thanks!
>>>>>>>>>>>
>>>>>>>>>>> Tom Lany
>>>>>>>>>>> http://tomlany.net
>>>>>>>>>>>
>>>>>>>>>>> On 6/25/10 4:04 PM, // ravi wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> On Jun 22, 2010, at 11:16 PM, Tom Lany wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks for the note. I was the person who reviewed this
>>>>>>>>>>>>>
>>>> theme. If you want to leave the code for the pages file
>>>> alone, that is probably fine. I just suggested this as it is
>>>> what is commonly done in most WordPress themes.
>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>> Hello Tom,
>>>>>>>>>>>>
>>>>>>>>>>>> thanks to you, then, for your detailed review notes. I have
>>>>>>>>>>>>
>>>> fixed (to my knowledge) all the issues identified and posted
>>>> 0.9 of the theme for review (a minute ago). I did leave the
>>>> code for the pages file as is, but added a check to the common
>>>> code to not display date stamp for pages (as suggested in the
>>>> Codex).
>>>>
>>>>>>>>>>>> Additionally I have passed the theme XHTML and CSS through the
>>>>>>>>>>>>
>>>> W3C validator and cleaned it up so the only warnings/errors
>>>> that remain are the -moz, -webkit shadow and border radius
>>>> stuff. FYI, since my development blog is not visible outside, I
>>>> used one of my external blogs (http://0sum.org/). Various other
>>>> things have been fixed as well: esc_attr() for relevant PHP
>>>> generated HTML attributes, IE specific CSS, use of
>>>> register_sidebar_widget() was removed, a few hints for added
>>>> for the collapsibility of sidebar widgets, meta tags were made
>>>> single-line, capitalised stylesheet elements were made
>>>> lowercase, the PHP errors/warnings noted in the Trac ticket
>>>> were resolved, reference to ‘global $_POST’ was removed, an
>>>> icon that was not GPL (from pixel-mixer, as you correctly
>>>> noted) was replaced, 404 page title was set to standard title
>>>> size.
>>>>
>>>>>>>>>>>> I also made sure that the search widget works (please let me
>>>>>>>>>>>>
>>>> know if there is some use case that I am missing). I have not
>>>> yet created a print style for this theme. That will be
>>>> significant work, to be done right, and I will work on that
>>>> shortly.
>>>>
>>>>>>>>>>>> Regards,
>>>>>>>>>>>>
>>>>>>>>>>>> —ravi
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks for your submission!
>>>>>>>>>>>>>
>>>>>>>>>>>>> Tom Lany
>>>>>>>>>>>>> http://tomlany.net
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>> ---------- Forwarded message ----------
>>>>>>>>>>>>>> From: // ravi<ravi at g8o.net>
>>>>>>>>>>>>>> Date: Tue, Jun 22, 2010 at 10:58 PM
>>>>>>>>>>>>>> Subject: Re: THEME: Rachel - 0.8 (closed theme: not-accepted)
>>>>>>>>>>>>>> To: Edward Caissie<edward.caissie at gmail.com>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Great feedback, Edward. I will work on fixing the validation
>>>>>>>>>>>>>>
>>>> etc. Regarding icons, those links were copied over from an
>>>> older version and I have stripped most icons out of the
>>>> theme. I will double check and update the links or replace
>>>> the icons. The one place I disagree is regarding copying code
>>>> from single.php. I would rather not have duplicated code.
>>>>
>>>>>>>>>>>>>> Regards, --ravi
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Jun 22, 2010, at 10:52 PM, Edward
>>>>>>>>>>>>>>
>>>> Caissie<edward.caissie at gmail.com> wrote:
>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Please see the details on this ticket in Trac:
>>>>>>>>>>>>>>>
>>>> http://themes.trac.wordpress.org/ticket/8
>>>>
>>>>>>>>>>>>>>> Thank you,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The WordPress.org Theme Team
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>
>>> _______________________________________________
>>> theme-reviewers mailing list
>>> theme-reviewers at lists.wordpress.org
>>> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>>>
>>>
>>
>>
> _______________________________________________
> theme-reviewers mailing list
> theme-reviewers at lists.wordpress.org
> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>
More information about the theme-reviewers
mailing list