The first issue i have with the fact that classes and html elements are not used consequent is as following:

  • Global forum
  • Active topics
  • New & updated topics
  • Unanswered topics

Above have different html elements and different classes. The topic link for example. Is not consequent on the above pages.

  • On the Global / normal forum page, the topic link is rendered as:
    <a class="topic-title"><span class="forum-topic-title">topic title</span></a>
  • On the Active topics forum page, the topic link is rendered as:
    <a class="topic-title">topic title</a>
  • On the New and updated topics forum page, the topic link is rendered as:
    <a><span class="forum-topic-title">topic title</span></a>
    On this page our topic title a, doesn't even have a class.
  • Unanswered topic has the same setup as the new and update topics forum page

I hope that my point is clear on this one. We should have the same setup on every page.

CommentFileSizeAuthor
#17 naked_seven.zip33.62 KBMichsk
#17 advanced_forum_active_topics.txt17.11 KBMichsk
#17 advanced_forum_new_topics.txt15.97 KBMichsk
#17 advanced_forum_topic_list.txt15.81 KBMichsk
#17 advanced_forum_unanswered_topics.txt11.54 KBMichsk

Comments

Michsk’s picture

Michsk commented

Above i use the title example but this goes for all elements. The images used in the first <td> next to the title are also not consequent themed.

And personally i would remove the <br /> to display the description on another line. Just use a div around the title, this is much better for theming.

ryivhnn’s picture

ryivhnn commented

You wanted "consistant", consequence is something that happens as a result of something else :)

They might all be different because people may want to theme thhem differently. Alternately I could be misunderstanding you or just have been setting up sites differently as I'm not quite sure what these pages you're referring to are.

Michsk’s picture

Michsk commented

HA you are correct, that was the wrong word. I ment consistant. Offcourse people could / would need to be able to, theme them different. But then we should add another class to a consistent markup.

And for the views based pages, like uncommented topic page, the main div around the whole view page should have a id="forum" since we also wrap the global forum in a div with that id. That fixes some of the inconsistencies for me.

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented
Title: Be consequent with classes. » Check class consistency

Since the classes were added as needed, I'm not surprised they aren't consistent. I've been asking for a while for a themer to give the styles a good going over to find stuff like this that trips up themers. I'm a programmer, not a themer, and know just enough HTML/CSS to be dangerous. ;)

Michelle

Michsk’s picture

Michsk commented

Michelle, i think we can and will need to work on this one together than. I am not a programmer but a themer, so sounds like a good combo.

I will be on irc later today, maybe we can talk about it there some more.

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented

That would be great but my part of the together won't be until next week. I might have a bit of time in 12 hours for about 2 hours after that but then I will barely be touching the computer except to process photos for a while. I've got a camera club field trip in the morning then about 2 hours to catch my breath followed by a family photo shoot and then a family photoshoot on Sunday and my own family to photograph on Sunday as well.

If you find any other wierdness in the mean time, feel free to keep adding issues.

As for this specific issue, I think the problem is that the main forum page is done in code, and therefore under my control, whereas the other two are views. Views is probably the one adding the classes but I'd have to dig in to be sure on that.

Michelle

Michsk’s picture

Michsk commented

Above seems true, i did find some more. Will post that also. How would you see this? Do you want me to get my hands the templates? I can also write a doc with everything in it and then you will be able to implement it? Since i have never worked wit CVN the second option seems best in my eyes.

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented

You can do patches if you want. Otherwise just list the file with the problem and what the problem is and I'll take it from there.

Michelle

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented

Have you had a chance to look at this? I'm not going to call this a blocker but I would like to get any changes into Alpha 4, if possible.

Michelle

Michsk’s picture

Michsk commented

No i haven't. But i will do it this week.

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented

Thanks! (And thanks on the other issue, too, so I don't have to bump both up.)

Michelle

Michsk’s picture

Michsk commented

wow, still haven't done this. Will get back at this asap.

Michsk’s picture

Michsk commented

What would you suggest as a startingpoint? I'm thinking of getting my hands on the naked skin and then you can work from there??

Michsk’s picture

Michsk commented

I'm creating a new theme: Naked 7.

From that theme you can do what you want, but thats the easiest way for me to work. It's basically the naked theme but then upgraded (+new images). I'm gone post a lot in this issue to keep some kind of log.

advanced_forum.naked_seven.images.css

Why would we do url(../forum/images/? Just do url(images/ for the images.

advanced_forum.naked_seven.forum-list.tpl.php

Oeh, very ugly bug:

<?php if ($item->is_container): ?>
  <tr id="subcontainer-<?php print $item_id; ?>" class="forum-row <?php print $item->zebra; ?>  class="container-<?php print $item_id; ?>-child">
<?php else: ?>
  <tr id="forum-<?php print $item_id; ?>" class="forum-row <?php print $item->zebra; ?>  class="container-<?php print $item_id; ?>-child">
<?php endif; ?>

view advanced_forum_topic_list

Got shaken up a bit to make the things more consistent.

Michsk’s picture

Michsk commented

All main template are ready. Starting with the topic's template.

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented

Not really following you, but I'll take a look when you're done... :)

Michelle

Michsk’s picture

Michsk commented
StatusFileSize
new advanced_forum_unanswered_topics.txt11.54 KB
new advanced_forum_topic_list.txt15.81 KB
new advanced_forum_new_topics.txt15.97 KB
new advanced_forum_active_topics.txt17.11 KB
new naked_seven.zip33.62 KB

New theme and some changed views.

Michsk’s picture

Michsk commented

This basically fixes this issue.

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented
Version: 6.x-2.x-dev » 7.x-2.x-dev

Thanks! I'm getting ready to head out for the weekend so probably won't get to look at this until next week. If you're doing this in 7.x, I'll bump the version to that. Troky may have a chance to look at it before I do. It'll be fixed in both versions, anyway, so doesn't matter too much which one goes first.

Michelle

Michsk’s picture

Michsk commented

it's based on d6.

troky’s picture

troky commented
Version: 7.x-2.x-dev » 6.x-2.x-dev

Porting code to D7 is much easier than porting styles. I am definitely postponing this for 'after-alpha' release.

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented
Status: Active » Needs review

That's fine. I just can't look at it right now and thought I'd toss the ball to you to see if you had time.

@lasac: I do appreciate you doing this but my module time has been scarce lately and I really need to focus on getting Alpha 4 out soon as I can get back to it. I will look at it soon as I can.

Michelle

mcdruid’s picture

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

lasac - sorry, but as the priority now is to get a full release of AF 6.x shipped while D6 is still in any way relevant, I'm going to say that "class consistency" is a nice-to-have and therefore not something which is going to be included.

I doubt the existing users of the module would welcome changes to the classes which broke their custom themeing. Furthermore, any changes to the views in AF would have to be done for a very good reason.

As your contributions were not submitted as patches, I can't see what actual changes you were proposing (without doing diffs with your versions etc..) so I'm afraid I'm closing this as won't fix.

If you - or anyone else - are particularly keen to get any of these changes in, please re-open the issue and submit patches against the current git branch.

Michsk’s picture

Michsk commented

Im keen to get it in, but it will be in my personal themes then.