Posted by daneyuleb on October 3, 2008 at 7:17pm
| Project: | FriendList |
| Version: | 6.x-1.x-dev |
| Component: | User interface |
| Category: | task |
| Priority: | minor |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
Nice to see this module so actively being worked on!
The friendlist block seems to need some minor formatting work--the bullets beside the friend categories appear strangely indented with bullets with no entries. The general look in Garland is:
* friend -- you are not friends
*
* Add to friends
* fan -- you are not a fan
*
* Become a fan
I realize it could be tweaked via theme, but you might want to take a look at it to make it appear less jumbled and hard to read in its default state...maybe with spacing better spacing or a divider between friend categories, and bullets only on the options:
friend -- you are not friends
* Add to friends
fan -- you are not a fan
* Become a fan
Thanks for the already great module!
-Daniel
Comments
#1
Hmmm...the indents didn't show up in the last post, but you get the idea...:)
-Daniel
#2
Hi, it is because the menus are nested. I've thought about this as well. I also think that only the links should be a list menu. The item list with the class "friendlist-user-links" could possible be substituted with div tags and leave the "friendlist-user-links-multiple" as list menu.
What do you think Merc?
Attached is a picture of the current nested lists.
Marius
#3
Hi,
Mine shows a bit different than your screen grab, for what it's worth. Each bullet is on its on line, not bullet 2 and 3 together as your screen grab shows. (That didn't look too bad, actually, compared to the result I'm getting). Here's what I see (attached).
-Daniel
#4
Is that Garland? My grab was from Minelli, which is exactly like Garland with fixed with. In any case, yours looks worse because of the margin and padding that gets added twice in a nested list. One more reason why we should avoid nesting.
We will look for a solution regarding this issue.
Marius
#5
Uhmmm, yeah...so I looked at this and don't actually know how to make it so it doesn't nest. Merc, would you know how we can achieve this?
Regards,
Marius
#6
Hi,
I know HTML 1.0 -- that's pretty much it :-/
Marius, maybe an entry in the Social Networking forums would help here?
In my very modest opinion, nested LIs is the way to go, because it's what seems to make the most sense in terms of HTML (structure wise).
It is a list in a list after all... isn't it?
Maybe we just need to tell people how to make it look prettier?
I am reluctant in turning the description (friend -- you are not friends) into an heading.
Bye,
Merc.
#7
Yes...this is also why I left it initially when I ran into this issue. In the end it _is_ a style issue, which should be solved at a style level, thanks for underlining that. I suggest we create an example snippet. If it remains to difficult for people (css theming), we should consider including a tiny ccs sheet for module ui. That way we can even have some commented code snippets in the stylesheet that people can uncomment to activate e.g. image replacement snippet.
Regards,
Marius
#8
Hi,
Are you technically able to do the style part Marius? Because I am not :-D
I will pay a graphic designer to do that if neither of us can.
Bye!
merc.
#9
It will be easy for me...I only don't know how to make the include, module wise. If you can give an empty .css file that module will load, I can do the rest.
Regards,
Marius
#10
Hi,
Question: is it sane to change the CSS? Won't it tend to break various themes etc?
Is it better to just give people instructions on how to make this look better?
I've seen so many messages around with CSSes broken by extra modules...
Merc.
#11
Yes, absolutely :)
Maybe, but we _should_ be able to make it so it doesn't. Basically we set 'list-style: none' together with margin and padding to '0' for the first list, so that it doesn't get added twice for the second list. Since good classes are provided for each list, we can do this without breaking themes.
I think it would be a valid solution to make it solid 'out of the box'. We potentially could get a lot of reported issues on this for those not comfortable with css and theming. I believe the best way to solve it is with css.
I would say lets do it with css...if complaints arrive about themes being broken we can always intervene with an alternative solution.
Does this convince you? I'm very comfortable with css, it is all I do for my site designs.
Regards,
Marius
#12
Hi,
I am totally convinced. Please add in CVS the CSS file that needs to be added, and I'll change the module so that it's added!
Merc.
#13
Hi Merc...to make it fool proof we need to find a way to add a class attribute to the 'li' items of the 'ul' list. Specifically the list for 'friendlist-user-links', but not 'friendlist-user-links-multiple'. Can we do that from this function?
<?php
/**
* An HTML-formatted list of links for EVERY relation type for
* the user. This function is used by the hook_user() hook
* to show links about the user.
*
* @param $account
* The user object (or id) you want the links for.
*
* @return
* HTML items, formatted with theme_item_list.
*/
function friendlist_ui_user_links($account) {
global $user;
$account = friendlist_api_user_into_uid($account);
// The user is "self": return nothing.
if ($user->uid == $account) {
return '';
}
$links = array();
// No permissions: nothing to do.
if (!user_access('have relationships with friendlist')) {
return '';
}
// Load the relation types in $relation_types.
$relation_types = friendlist_api_relation_types_load_all();
// Cycle the active relation types, and
// add links as appropriate.
foreach ($relation_types as $rt) {
if ($rt->active) {
$status = friendlist_api_relation_status_get($user, $account, $rt->rtid);
$status_data = friendlist_api_status_data($status);
$links[] = '<span class="friendlist-relation-name">'. t($rt->name) .'</span> — <span class="friendlist-relation-info">'. t($status_data['description'], array('!rt_name' => $rt->name, '!rt_name_p' => $rt->name_p)) .'</span>';
$l1 = friendlist_ui_get_link('add', $account, $rt->rtid);
$l2 = friendlist_ui_get_link('delete', $account, $rt->rtid);
$sublist = array();
if ($l1) {
$sublist[] = $l1;
}
if ($l2) {
$sublist[] = $l2;
}
$links[] = theme('item_list', $sublist, NULL, 'ul', array('class' => 'friendlist-user-links-multiple'));
}
}
// Return the themed links IF they are.
if (count($links) != 0) {
return theme('item_list', $links, NULL, 'ul', array('class' => 'friendlist-user-links'));
}
// No links to be returned!
return '';
}
?>
It would be in regard to 'li' item generated by these themed links:
<?php// Return the themed links IF they are.
if (count($links) != 0) {
return theme('item_list', $links, NULL, 'ul', array('class' => 'friendlist-user-links'));
}
?>
This would ensure 100% consistency, e.g.
<li class="friendlist-user-list">. Otherwise it would stay as a makeshift solution.Is that possible? I studied the API theme_item_list, but it didn't offer insight for me.
Regards,
Marius
#14
Hi,
I think it's actually dead simple: there is a simple parameter:
$attributes The attributes applied to the list element
So... it should be easy.
I am not sure why you pasted the code above... did you modify it? :-D
I committed the changes.. Now, each LI has a class. Please let me know if this is what you expected :-D
If you still have problems with CVS, heree is what I changed:
<?phpif ($l1) {
$sublist[] = array('data' => $l1, 'class' => 'friendlist-user-list-item'); # CHANGED!
}
if ($l2) {
$sublist[] = array('data' => $l2, 'class' => 'friendlist-user-list-item'); # CHANGED!
}
?>
Bye!
Merc.
#15
Hi Merc...I pasted the code to see the context of the whole function :), so you wouldn't have to look it up. In any case, your solution does indeed add classes, but for the nested list. I need classes for the primary list items. Does that makes sense? Would it work to change it to this:
<?php$links[] = array('data' => '<span class="friendlist-relation-name">'. t($rt->name) .'</span> — <span class="friendlist-relation-info">'. t($status_data['description'], array('!rt_name' => $rt->name, '!rt_name_p' => $rt->name_p)) .'</span>', 'class' => 'friendlist-user-list-item');
?>
I'm basically following your example, but for the $links array rather than $sublist. I don't know if that will work?
Regards,
Marius
#16
Hmmm....that arrays looks like a big mess. It semi works, but variables are not passed. It is only slightly over my head, I might be able to figure it out by trial and error, but you can probably do it by just blinking with your eyes.
Regards,
Marius
#17
Hi,
Here's the code you want:
<?php$links[] = array('data' => theme('item_list', $sublist, NULL, 'ul', array('class' => 'friendlist-user-links-multiple')) ,
'class' => 'friendlist-user-link-item'
);
?>
Committed!
Does it add up? :-D
Merc.
#18
Hi merc...I'm sorry, it doesn't just yet. Please bare with me...
There are two ul available lists:
- ul with class 'friendlist-user-links' (this is the primary list)
- ul with class 'friendlist-user-links-multiple' (this is the nested list)
In order to style it properly, I need the primary list, 'friendlist-user-links', to have classes for its _li_
However, I need the _li_ for the nested list, 'friendlist-user-links-multiple', to stay without a class (or at least a different class than the primary list).
I hope it makes more sense now. If not I can always take pictures :)
Regards,
Marius
#19
Hi,
Rather than pictures... can you tell me what the HTML output looks like _now_, and what it _should_ look like instead?
Bye!
Merc.
#20
Hi Merc, below is the HTML for the lists. I've stripped some of the unnecessary stuff to show the lists better. There are 4 list items. In list item 2 and 4 of the first list are nested list for ul class "friendlist-user-links-multiple". The list items in these nested lists should not have a class, but the list items 1, 2, 3 and 4 should have a class, e.g. friendlist-user-list-item.
So basically list item class attributes for the primary list, but not for the nested list.
<?php
<div class="item-list">
<ul class="friendlist-user-links">
<li class="friendlist-user-list-item first"> <!-- LIST ITEM 1 -->
<span>friend — You are not friends</span>
</li>
<li class="friendlist-user-list-item"> <!-- LIST ITEM 2 -->
<div class="item-list">
<ul class="friendlist-user-links-multiple">
<li>
<a href="/friendlist/add/3/1?destination=user/3">Add to friends</a>
</li>
</ul>
</div>
</li>
<li class="friendlist-user-list-item"> <!-- LIST ITEM 3 -->
<span>fan — Youre not a fan</span>
</li>
<li class="friendlist-user-list-item last"> <!-- LIST ITEM 4 -->
<div class="item-list">
<ul class="friendlist-user-links-multiple">
<li>
<a href="/friendlist/add/3/2?destination=user/3">Become a fan</a>
</li>
</ul>
</div>
</li>
</ul>
</div>
?>
I hope this helps.
Regards,
Marius
PS. I just had to attach a picture, so you can see what the actual list looks like on the profile. It might help putting it in context.
#21
Hi,
Sorry Marius, I am getting very confused... and it's my fault!
But... I don't suppose you could add a note with:
* What comes out *now*
* What's *supposed* to come out?
Thanks a billion,
Merc.
#22
Hi Merc...here's yet another stab at it ;)
ORIGINAL (including your latest list commits):
<?php
<div class="item-list">
<ul class="friendlist-user-links">
<li class="first">
<span class="friendlist-relation-name">friend</span> — <span class="friendlist-relation-info">You are not friends</span>
</li>
<li class="friendlist-user-link-item">
<div class="item-list">
<ul class="friendlist-user-links-multiple">
<li class="friendlist-user-list-item first last">
<a href="/friendlist/add/4/1?destination=user/4" class="popups friendlist_ui_TW_NONE_add">Add to friends</a>
</li>
</ul>
</div>
</li>
<li>
<span class="friendlist-relation-name">fan</span> — <span class="friendlist-relation-info">Youre not a fan</span>
</li>
<li class="friendlist-user-link-item last">
<div class="item-list">
<ul class="friendlist-user-links-multiple">
<li class="friendlist-user-list-item first last">
<a href="/friendlist/add/4/2?destination=user/4" class="popups friendlist_ui_OW_NONE_add">Become a fan</a>
</li>
</ul>
</div>
</li>
</ul>
</div>
?>
WHAT'S NEEDED:
<?php
<div class="item-list">
<ul class="friendlist-user-links">
<li class="friendlist-user-list-item first">
<span class="friendlist-relation-name">friend</span> — <span class="friendlist-relation-info">You are not friends</span>
</li>
<li class="friendlist-user-list-item">
<div class="item-list">
<ul class="friendlist-user-links-multiple">
<li class="first last">
<a href="/friendlist/add/4/1?destination=user/4" class="popups friendlist_ui_TW_NONE_add">Add to friends</a>
</li>
</ul>
</div>
</li>
<li class="friendlist-user-list-item">
<span class="friendlist-relation-name">fan</span> — <span class="friendlist-relation-info">Youre not a fan</span>
</li>
<li class="friendlist-user-list-item last">
<div class="item-list">
<ul class="friendlist-user-links-multiple">
<li class="first last">
<a href="/friendlist/add/4/2?destination=user/4" class="popups friendlist_ui_OW_NONE_add">Become a fan</a>
</li>
</ul>
</div>
</li>
</ul>
</div>
?>
I hope it is getting more clear :(
Regards,
Marius
#23
Hi,
Actually...
I redid from scratch the way the user's page is rendered.
The previous way, with two nested lists, was... err... I don't know. I must have been drinking. Something very alcoholic.
I think this is fixed now. Marius, I think the HTML makes a _lot_ more sense now.
What do you think?
Merc.
#24
Hi Merc...that's much better, thanks for reworking that code. I think its safe to close this now...the included css file also becomes obsolete with the current code.
Regards,
Marius
#25
Hi,
Fantatic!!!
(And sorry for wasting your time... :-/ )
Merc.
#26
Automatically closed -- issue fixed for two weeks with no activity.