buddypress / next-template-packs

is this the next BuddyPress template pack?
35 stars 9 forks source link

CSS .hidden problem #181

Closed r-a-y closed 7 years ago

r-a-y commented 7 years ago

This is a problem that has existed in bp-legacy and I'm seeing the same problems here as well.

Themes / plugins using the .hidden CSS declaration conflicts with some UI components in BuddyPress, which also need to be solved in Nouveau.

Related tickets:

hnla commented 7 years ago

Surprised by that where are you seeing the use of ,hidden I thought I changed any instances to bp-hidden or something similar.

r-a-y commented 7 years ago

It's not strictly a Nouveau problem.

Try using a WordPress theme powered by Bootstrap such as WP Bootstrap Starter theme or latest bbPress 2.6 RC.

Both use the .hidden CSS class.

hnla commented 7 years ago

Yep spotted them on activity comments - it might have to be we strip the class out from Nouveau comments as we don't actually use the class but JS to hide them, but it is more a core issue where we need to update to .bp-hide & add a new ruleset to core styles.

r-a-y commented 7 years ago

I just removed the hidden class for activity comments. I don't think we need a dedicated hidden class in this instance.

hnla commented 7 years ago

Oh cra** that's just what you've done in the commit I didn't bother to read. :)

thanks for the fix r-a-y - works for me, although may change the use of jquery .hide() to setting of classes.

hnla commented 7 years ago

also likely needs an aria role expanded on the link I'll re-open the roles ticket for that.

hnla commented 7 years ago

@r-a-y I fretted a little as noted there were still classes .hidden being added to those li elements, rather than remove all I decided to add back a class initially but as .bp-hidden and remove it on click event from li elements, partly as there is the sub routine I don't quite get that is showing the last child li element if not more than 5 comments or something & checking for .hidden so rather than break stuff changed all class strings.

If this looks odd can you comment and I'll revert out changes.

r-a-y commented 7 years ago

Not sure what the use case for adding a class on an element that is supposed to be hidden is. If you cannot see it, you cannot style it properly.

You even note in the commit message that the .bp-hidden class isn't being used.

hnla commented 7 years ago

@r-a-y valid points - then need to remove those classes and re-factor that odd block looking for the hidden classes & it's that that causes me issues, if I had the inclination I'd re-write the entire thing tbh.

hnla commented 7 years ago

@r-a-y sorry to impose on you but can you check line 359 as the logic of it makes no sense to me it seems to suggest that there could be some issue with counting iterations - 5 and hiding I just feel the whole approach to the initial count is wrong it also hides the first comments rather than maybe more logically the last ones we tend to read from first to last after all I want to see the initial comments rather than comments that may look orphaned as they are referencing hidden comments.

hnla commented 7 years ago

Actually forget it I'm going to leave as is the whole thing distresses me :) & would re-write, but as is for that 'reveal last item' code block it requires some form of token to id against visible items, & I don't have time to figure out a better re-write.

hnla commented 7 years ago

Closing - re-open if issue still exists.