boonebgorges / bp-groupblog

BuddyPress Groupblog
GNU General Public License v3.0
15 stars 10 forks source link

Duplicate IDs #38

Closed christianwach closed 9 years ago

christianwach commented 9 years ago

@boonebgorges This is a bit of an odd one... so I'm asking your advice before submitting a PR.

The basic issue is that BP Groupblog produces two nav items with identical IDs by default:

Both of these are wrapped by <li> elements with identical IDs too, i.e. #group-blog-groups-li.

The effect of this is that on a default CBOX install, the Group Admin Menu item is styled by the same CSS intended for the Group Nav Menu item, resulting in an IconSweets icon being inserted before the link and throwing the Group Admin Menu out.

Given that the Group Admin Menu item's values are auto-generated by BuddyPress, it looks like the easiest nav item to amend would be the one in the Group Nav Menu. Most other links in that menu are prefixed by nav-, which seems reasonable. However, doing this will break the CSS applied to it by the CBOX theme - and probably by other themes too.

So, which route to choose? Long term, I favour changing the Group Nav Menu item. Do you have any thoughts on this?

boonebgorges commented 9 years ago

Thanks for the explanation. I have fixed this in stylesheets before, but it'd never dawned on me that it was a problem with duplicate IDs.

Agreed that it should be fixed in the Group Nav Menu. Things will break, but they're already broken :)

On 03/17/15 08:18, Christian Wach wrote:

@boonebgorges https://github.com/boonebgorges This is a bit of an odd one... so I'm asking your advice before submitting a PR.

The basic issue is that BP Groupblog produces two nav items with identical IDs by default:

  • A link with ID |#group-blog| in the Group Nav Menu
  • A link with ID |#group-blog| in the Group Admin Menu

Both of these are wrapped by |

  • | elements with identical IDs too, i.e. |#group-blog-groups-li|.

    The effect of this is that on a default CBOX install, the Group Admin Menu item is styled by the same CSS intended for the Group Nav Menu item, resulting in an IconSweets icon being inserted before the link and throwing the Group Admin Menu out.

    Given that the Group Admin Menu item's values are auto-generated by BuddyPress, it looks like the easiest nav item to amend would be the one in the Group Nav Menu. Most other links in that menu are prefixed by |nav-|, which seems reasonable. However, doing this will break the CSS applied to it by the CBOX theme - and probably by other themes too.

    So, which route to choose? Long term, I favour changing the Group Nav Menu item. Do you have any thoughts on this?

    — Reply to this email directly or view it on GitHub https://github.com/boonebgorges/bp-groupblog/issues/38.

  • christianwach commented 9 years ago

    Thanks Boone, I'd say let's do it then! I've opened a PR here and will do the same at the CBOX theme repo.