If we are viewing a page with no title (ex. a frontpage), the ctools content type page_title prints an empty h1 tag.

<h1> </h1>

Besides just being semantically bad and possibly bad for SEO (BLACK MAGIC), it is a problem for our themer. We have margin attached to page title h1's and this is applied even though we don't want the space.

Our solution has been to create a seperate content_type that is smarter about that (and traditionally avoids the ctools token system.)

CommentFileSizeAuthor
#3 ctools-711922-2.patch596 bytesjonskulski
#1 ctools-711922.patch511 bytesjonskulski

Comments

jonskulski’s picture

jonskulski commented
Status: Active » Needs review
StatusFileSize
new ctools-711922.patch511 bytes

Here is a possible solution. Though this problem extends to the other page element content_types I am told. If accepted, I will patch as necessary.

Thanks!

merlinofchaos’s picture

merlinofchaos commented

You should do this against -dev, as that code has changed a little.

jonskulski’s picture

jonskulski commented
StatusFileSize
new ctools-711922-2.patch596 bytes

My mistake. Rerolled!

merlinofchaos’s picture

merlinofchaos commented
Title: Page Title content type outputs empty <h1> tags if there is no title » Page Title hardcodes to H1
Status: Needs review » Active

I committed this; I almost didn't, because I got to thinking that we're hardcoding an H1 in there, and that's never the right answer. That really should be an option or something. But then I realized that was no reason to keep this needed bug fix from going on. So I'm re-activating this issue as a reminder that H1 should probably be selectable. Perhaps a select list of tags to use.

jonskulski’s picture

jonskulski commented

A select list seems limited. What about a theme function here?

merlinofchaos’s picture

merlinofchaos commented

Possibly, though that means messing with theme functions for something that is very very simple. Hmm.

jonskulski’s picture

jonskulski commented

I am hesitant to use a theme function here too, even though I think it's the 'Right' solution. Howver, now that panels are themable having a theme function or option here doesn't gain us any ground except in style points.

An option list, or text box, would open a flood gate. There would need to be options for other panels as well. An option list would be limited and would stop being useful for anything but very simple cases, i.e. if we needs to wrap a span around it then we would have to theme that panel anyway.

A textarea would just be duplicating the functionality of the theme templating system. It is a slightly more palatable solution though.

merlinofchaos’s picture

merlinofchaos commented
Status: Active » Fixed

The current page title is much nicer about the tag and lets you select from several choices.

Status: Fixed » Closed (fixed)

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

jwilson3’s picture

jwilson3
Primary language English
Location Ecuador
commented
Issue summary: View changes
Related issues: +#2265451: Hide empty page title pane when wrapper tag is used

The current page title is much nicer about the tag and lets you select from several choices.

This is true, however the original problem reported here still stands with today's page_title pane. If you select an H1 and there is no page title, then an empty <h1></h1> is still printed.

I had opened up a separate issue #2265451: Hide empty page title pane when wrapper tag is used about this a while back... and now hit this problem again. In searching for that patch, I found this issue. Due to the margin on the empty h1, we get unwanted whitespace when there shouldn't be a page title.

Since there are patches on that issue that work on latest 7.x dev branch, can we continue there or should I reopen this and mark #2265451 as a duplicate?