Opened 8 years ago
Closed 7 years ago
#7443 closed defect (bug) (fixed)
Hidden groups CSS specificity
Reported by: | MikeGillihan | Owned by: | r-a-y |
---|---|---|---|
Milestone: | 2.9 | Priority: | normal |
Severity: | minor | Version: | 1.7 |
Component: | Groups | Keywords: | has-patch |
Cc: |
Description
The "hidden" class applied to a Hidden group in the Groups list is too generic. The .hidden pattern is fairly widespread and anytime a user with that pattern in a theme/plugin tries to view their groups, the Hidden groups will not be displayed appropriately.
While I don't really think it is a bug, it doesn't appear that there is a presentational reason for having that class so changing it to something like "hidden-group" would be more appropriate IMHO and eliminate the conflicts.
Attachments (1)
Change History (14)
#1
@
8 years ago
- Component changed from Core to Groups
- Keywords dev-feedback added
- Type changed from enhancement to defect (bug)
- Version changed from 2.7.2 to 1.7
This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.
8 years ago
#3
@
8 years ago
@r-a-y, @hnla
IMHO, this is only an expectation "we'll probably break sites that are already using this rule" and is at least at the same level as breaking sites using the actual default rule (and the reason of this ticket).
Would you name a class .relative
or .width
? Probably not. So why would you continue to use .hidden
?
This shouldn't be a long decision, especially because it exist a CSS naming convention.
#4
@
8 years ago
So @danbp you're arguing to change the class altogether? and be damned those that chose hang styles on it, inclined to not disagree but on the fence.
How about a further option:
retain the class 'hidden'
to retain styles hooked already to it, add a new class '.bp-group-hidden'
to that add the styles we need with a little specificity if necessary to ensure the properties are favoured, not not perhaps ideal but...
I'll go with the consensus opinion on the fix, whilst running over to Nouveau templates to try and make sure it doesn't occur there :)
#5
@
8 years ago
If a theme is using .hidden { display: none; }
would !important be absolutely necessary in an upstream style sheet? I think greater specifity would be enough. For example #group .hidden { display: block; }
#6
@
8 years ago
Thanks for the quick replies.
I'm not sure anything other than removing/replacing the hidden class would resolve this ticket, but I don't think I could call this a bug in BP either. In my opinion, the CSS pattern itself is pretty shortsighted and shouldn't be used, especially with the !important tag.
In the project I'm working on I added a quick override using !important because the theme employs many of H5BP's styles. I'm pretty sure bootstrap used the same pattern at some point so I figured tracking the issue was worth it.
Not being knee deep in BuddyPress I'm not sure what ramifications removing the class would have functionally. It seems to be a bit of a damned if you do, damned if you don't situation.
Again, thanks for the response and listening to me ramble. :)
#7
@
8 years ago
- Keywords close added
@MikeGillihan Thanks very much for the report - I've run into this before as well. As you note, it's bad practice - both by BP and by the theme that applies !important
rules to .hidden
elements.
I'll go with the consensus opinion on the fix, whilst running over to Nouveau templates to try and make sure it doesn't occur there :)
Unfortunately, I think this is probably the best we can do, without breaking existing sites in ugly ways.
#8
@
7 years ago
- Keywords has-patch added; dev-feedback close removed
- Milestone changed from Awaiting Review to 2.9
Moving this to 2.9.
Bootstrap uses the .hidden
CSS class, which conflicts with the .hidden
class we use in group lists.
I would recommend the following CSS declaration to fix this:
#groups-list li.hidden { display: list-item !important; }
This would probably minimize the amount of breakage, however it might cause problems in custom BP themes that are using display: inline-block
or some other variation.
In that case, I've also attached a JS fix for this.
Related support thread:
https://buddypress.org/support/topic/hidden-groups-and-forums-not-showing-for-members-of-them/
Yeah, I can see the
.hidden
CSS rule being used by other themes here.If we make the change to the
hidden
CSS class to something else likehidden-group
, we'll probably break sites that are already using this rule to style hidden groups in Groups loop.I think this is important to fix though.
We could do what other people have done in the support thread and that is to use a
display
CSS override using!important
. This wouldn't break any sites, but I know @hnla will hate this option :)