Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 5 years ago

#6088 closed enhancement (fixed)

Twenty Fifteen pagination styles adds big black square to pagination links of all BP Directory Pages

Reported by: mercime Owned by:
Milestone: 2.3 Priority: normal
Severity: normal Version:
Component: Appearance - Template Pack Keywords: has-patch close
Cc: mercijavier@…

Description (last modified by mercime)

WP 4.1, BP 2.2 latest trunk. Naturally happens when you have more than one page to any groups, members, or blogs directory.

Attached screenshot for your reference.

Another ticket coming up for joined group avatars misaligning in Activity Stream when Twenty Fifteen theme is activated. These issues bring to mind the references I've seen in some trac tickets to create specific stylesheets for the WP default themes. Maybe it's time to start on Twenty Fifteen theme :)

Attachments (5)

directory-navigation.png (77.7 KB) - added by mercime 5 years ago.
big black box in pagination
6088.patch (1.1 KB) - added by mercime 5 years ago.
pagination-fix.gif (24.1 KB) - added by mercime 5 years ago.
Fixed BP pagination in Twenty Fifteen
6088.02.patch (1.4 KB) - added by r-a-y 5 years ago.
pagination-603-below.gif (34.7 KB) - added by mercime 5 years ago.

Download all attachments as: .zip

Change History (25)

@mercime
5 years ago

big black box in pagination

#1 @mercime
5 years ago

  • Description modified (diff)

#2 @mercime
5 years ago

  • Component changed from Core to Template Pack
  • Keywords has-patch added
  • Type changed from defect (bug) to enhancement

Simple fix for the pagination styles added in Twenty Fifteen. The fix has no adverse effects on the other WP Default Themes (2010-2014) per my review.

Attached patch and an animated gif

Last edited 5 years ago by mercime (previous) (diff)

@mercime
5 years ago

@mercime
5 years ago

Fixed BP pagination in Twenty Fifteen

#3 @mercime
5 years ago

  • Description modified (diff)

#4 @DJPaul
5 years ago

  • Milestone changed from Awaiting Review to 2.2

This ticket was mentioned in Slack in #buddypress by mercime. View the logs.


5 years ago

@r-a-y
5 years ago

#6 @r-a-y
5 years ago

02.patch is based off of mercime's patch with further adjustments to the pagination height and the removal of box-shadow and border-bottom.

Tested on twentytwelve as well and is non-destructive.

#7 @mercime
5 years ago

  • Cc mercijavier@… added

#8 @boonebgorges
5 years ago

Changes seem safe enough to me. If everyone else is good with it, I'm fine with this for 2.2.

#9 @mercime
5 years ago

Boone, since we're building a new Twenty-Fifteen-specific-stylesheet per our discussion in Slack, the style fixes would be extraneous code for the other Twenty-* themes which don't have this BP-compat style issue, among others, found in Twenty Fifteen.

We only need to set a deadline for the contributors who said they have a bp-specific stylesheet for the Twenty Fifteen theme when they met with Paul and JJJ in WCSF last October. On my end, I have completed around 50% of the twentyfifteen.css (because I had already started doing it before) and sent it to hnla who's ready to complete it if the contributors do not meet deadline.

#10 @hnla
5 years ago

Can we consider this ticket closed and focus in the other ticket #6124 to avoid confusion, I think all patches here are now ported over to that ticket, including @r-a-y 02 patch ?

#11 @DJPaul
5 years ago

The other ticket is still under discussion and we won't be introducing any theme-specific stylesheets into 2.2, because it's just too late for new features to be added to 2.2.

Since everyone involved in this ticket has said these changes are needed to improve our current templates' theme compatibility with all themes, let's include these in 2.2 as planned, and see where the other ticket takes us for 2.3.

#12 @boonebgorges
5 years ago

Since everyone involved in this ticket has said these changes are needed to improve our current templates' theme compatibility with all themes, let's include these in 2.2 as planned, and see where the other ticket takes us for 2.3.

Agreed. If and when #6124 is completed, we could yank the styles currently under consideration out of buddypress.css and into twentyfifteen.css, if people liked that idea. hnla and mercime - is this plan OK with you?

#13 @mercime
5 years ago

Sure Boone, no problem. The BP elements just look unsightly in the Twenty Fifteen theme on desktop or mobile, I'll just continue to use Twenty Fourteen in screenshots.

Thank you for the patch r-a-y. Works in desktop, but missing a number or so if on mobile per attached. [edit] I suppose we'll leave that to the designers who will be creating the 2015-specific BP stylesheet.

Last edited 5 years ago by mercime (previous) (diff)

#14 @hnla
5 years ago

we could yank the styles currently under consideration out of buddypress.css and into twentyfifteen.css, if people liked that idea. hnla and mercime - is this plan OK with you?

I think we need to be very careful here, adding theme specific styles to our generic stylesheet is not a good idea, this sheet's purpose is to deal with styling of BP elements not styling of BP elements to look good for a given theme.

Adding styles only to then on subsequent releases attempt to extract them again is simply messy. If we're not sure yet about #6124 - and I can appreciate the sentiments expressed therein - then only approach we should follow strictly'' in this ticket is identifying those properties that can and perhaps should have been added or removed.

As an example, twentyfifteen takes the decision to style anchors with 'border-bottom' this we could have anticipated and allowed for in our nav related rulesets and is a property we know we do not want declared for our object-nav, item-list tabs, so adding a property/value of ? .item-list-tabs a {border-bottom: none;} is something that can safely exist in our default styles and need not be re-visited; conversely adding a ruleset to, say, cover decreasing a heading size is something we should not and can not do in our generic styles.

If we are adding styles then to our generic sheet we must be very careful to only add styles that we are not going to need to extract at a later date, imho.

Long ago, lost in the mists of time when the browser war battles raged it came to pass that people referred to Hacking their styleseets to accommodate and fix browser variances, this approach later came to be frowned on; the term - used erroneously - hacking was deemed to be in-appropriate and in fact what people were doing was Filtering styles, at the same time Best Practises stated that a stylesheet follow a number of dictates one of those being maintainability and purity of styles thus mixed styles where there were any number of hacks based on browser errors in parsing values correctly e.g the Star Hack. Due to the inescapable fact that we had to use these the common practise was to include these - where possible - in separate sheets, allowing for easy removal when/if no longer required - that in principle then is what we propose in adding theme specific Fixes where we can't address them in the main styles as non destructive styles for all themes.

This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.


5 years ago

#16 @johnjamesjacoby
5 years ago

Mostly agree with Hugo that we shouldn't start littering the default styling with any rules that could collide with unintended themes. I think what's on #6124 is a more logical approach to solving this problem, and lets us work towards improving support for all bundled themes in the bp-legacy template pack.

This ticket was mentioned in Slack in #buddypress by djpaul. View the logs.


5 years ago

#18 @DJPaul
5 years ago

  • Milestone changed from 2.2 to Future Release

Decided to move these out of 2.2 pending consideration of #6124 for 2.3.

#19 @mercime
5 years ago

  • Keywords close added
  • Milestone changed from Future Release to 2.3

Closing this ticket which was superceded by #6124 Twentyfifteen BP Companion Stylesheet and fixed in #6291 Companion stylesheet task - twentyfifteen

#20 @mercime
5 years ago

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.