Skip to:
Content

Opened 5 years ago

Closed 5 years ago

Last modified 19 months ago

#4953 closed enhancement (fixed)

BuddyPress specific UI refresh

Reported by: karmatosed Owned by: boonebgorges
Milestone: 1.8 Priority: normal
Severity: normal Version:
Component: Core Keywords: has-patch needs-testing
Cc: karmatosed@…, mercijavier@…, trisha@…

Description

There are a number of specific UI elements that every theme needs. The current version in 1.7 has a legacy styling which could do with a refresh and any new template will need to also have. They aren't something that can be inherited totally from the theme so are still applicable.

The suggested list to be considered are:

  • Messages (error, success, notices) : particularly checking contrast levels on this to make sure we are accessible
  • Buttons (comment, reply, favourite) - ideally would use the theme .button style for colour and not have a gradient/radius
  • Span number indicators
  • Loaders

There are probably other areas that could be added to here so if anyone knows any we could add them.

There is another ticket that suggests a new template pack (http://buddypress.trac.wordpress.org/ticket/4952). If this ticket was done for 1.8 then this could be considered part of that, however if it was to be potentially for 1.9 or 'when ready' then having this as part of 1.8 could also 'detheme' a bit more the legacy templates by adding in the following:

  • Making sure fonts and font sizes come from theme (px to rem would also be good)
  • Remove all border radius, gradients, drop shadows (if any)

Whatever is done here would be needed for the new template pack so a good easy step towards that, but it could be released before to have something in 1.8 UI wise and also clean up legacy (people are going to use it for a while).

A simple plugin could be developed for this to get easier testing, but it could also be done just in CSS - depends if want in core from start or to take a mp6 approach.

Attachments (7)

screen-buttons.png (20.3 KB) - added by karmatosed 5 years ago.
screen-error-2.png (15.5 KB) - added by karmatosed 5 years ago.
screen-notice.png (5.2 KB) - added by karmatosed 5 years ago.
screen-success.png (11.9 KB) - added by karmatosed 5 years ago.
style.diff (14.0 KB) - added by karmatosed 5 years ago.
style-second.diff (14.7 KB) - added by karmatosed 5 years ago.
4953.patch (15.3 KB) - added by boonebgorges 5 years ago.

Download all attachments as: .zip

Change History (33)

#1 @ubernaut
5 years ago

Pardon me for being slow but I'm having trouble following exactly how this "de-theming" is supposed to work. If we are trying to pull styles and attributes from any given theme doesn't that require some kind of standard (or way to universally recognize the) set of the pertinent id's classes and selectors?

Or are we providing a set of id's classes and selectors which they modify with the own sub- id's classes and selectors?

Or do i not understand at all whats happening here?

Ready to kill tasks as soon as i know what needs to be done exactly. Thanks!

#2 @hnla
5 years ago

@ubernaut not sure what 'universally recognize' means essentially bp legacy or packs must be careful not to impose any styles that aren't positional, structural or of a necessary nature for the rendering of the element dtheming in this scenario simply means running through the rulesets identifying properties such as background/gradients and removing but equally looking at BP specific unique ui aspects and updating the styles where they would benefit and didn't cause a clash with unknown themes styles ( the difficult aspect) There is a pseudo namespace token in the form of #buddypress that acts as the primary top level ancestral elements token so all bp elements fall under that element.

Making sure fonts and font sizes come from theme (px to rem would also be good)

Perhaps the hardest aspect, any set font-size needs to be a relative measure unless one dictates that e.g activity timestamps need to be 'small' and set fixed pixel size (we haven't a clue what a rem value might actually end up being) testing how the use of keywords fairs might be worthwhile (historically terrible x browser and generally problematic) At the end of the day and as much as one wants to manage sizing on aspects we know must be smaller/larger it may be safest and most practical to leave font properties out altogether.

There is an issue as I see it but may stand to be corrected; with the legacy template files having got out in the wild any changes made to template markup and or styles could impact sites based on modifications of those, or is there a mechanism proposed that switches i.e not been aware of this 'plugin' aspect for packs or how that work, select plugin pack one wants and activate?

On a sidenote but related I have almost finished re-factoring the /members/single/*.* templates to use the newer bp_nav_menu() and removing the bp_get_options_nav() altogether from sub templates along with a set of styles to provide basic structuring of a true nested menu ul structure to mimic what visually rendered in existing screens. It has a enqueue function for the styles and was intended as a drop in to themes pro tem, but may be worth gutting and introducing so that we make use of bp_nav_menu() moving forward.

#3 @ubernaut
5 years ago

@hnla thanks for clarifying and yeah i guess my questions stemmed from the fact that i had never actually had to use the template pack myself not being a theme developer i suppose i didn't understand that it's a given that anyone who utilized the pack has already utilized or modified the correct css elements. Also i didn't really understand what i meant either when i mentioned they universal way to recognize either my brain was trying to work backwards from a place of total confusion about the meaning of what i read in the ticket. :P

i would concur that modifying these existing (in the wild as you say) elements would seem at least to me to be nothing but trouble unless there is some aspect which know to cause issues in every case and should therefore be removed. If modernizing the template pack is the goal it certainly makes sense to me at least that a new template pack the way to go and should be separate from the existing legacy elements.

Again maybe my own personal level of experience with this process disqualifies me from any useful comment but i would like to help if i can so feel free to just tell me what i can do to help out. :)

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

#4 @boonebgorges
5 years ago

with the legacy template files having got out in the wild any changes made to template markup and or styles could impact sites based on modifications of those, or is there a mechanism proposed that switches i.e not been aware of this 'plugin' aspect for packs or how that work, select plugin pack one wants and activate?

It hasn't been 100% decided, but, like you, I'm very wary to make any changes to the in-the-wild bp-legacy templates that will break existing implementations. There are many things that we can and probably should do to the existing templates - since they were ported over pretty much directly from bp-default, and lord knows that there is much to be improved about bp-default's markup - but we shouldn't, for example, change the names of CSS selectors. Any "modernization" that is truly substantial - say, reworking the whole thing to use HTML5 elements - should be done in a separate template pack.

have almost finished re-factoring the /members/single/*.* templates to use the newer bp_nav_menu()

Awesome. This might need to go in a new template pack, unless bp_nav_menu() outputs the exact same markup as the old function (I haven't checked).

#5 @hnla
5 years ago

but we shouldn't, for example, change the names of CSS selectors. Any "modernization" that is truly substantial - say, reworking the whole thing to use HTML5 elements - should be done in a separate template pack.

Yes the principle must always remain that existing and historic tokens are preserved and not changed, new ones can be added but old not removed and in the main the existing ones serve well enough and given how we unhooked the JS selectors from the tokens we have gained the ability to re-factor markup without upsetting the scripting behaviour.

Substantial re-factoring will have to be reserved for new packs, we may be able to extract some of the markup from work done on the Status theme, as part of the scope Tammie and I tried to achieve there was html5 elemenst & lightening of structure.

unless bp_nav_menu() outputs the exact same markup as the old function (I haven't checked)

It's substantially different due to the now nested ul construct, however I tried to re-factor preserving the original outer elements wrapping bp_displayed_user_nav() to try and preserve any styles that might have relied on descendent styling, the new styles mimic the look pretty much of 1.7 running under 2012 so in effect may just work copied over existing, be interesting to test. Original thought was users try it out by extracting from a zip the /members/ dir and templates dropped into /buddypress/ in their theme along with snippet enqueue function and stylesheet - it's usable one way or another, fiddly issues are bogging things down but going to zip it when done so anyone can have a play.

@ubernaut no worries just had a notion that you were probably over-thinking the complexity slightly and that although not simple processes also not too hard, and anyone can get stuck in, also think all comments are valid, we all use the stuff so and have valid opinions or thoughts.

#6 @hnla
5 years ago

Although a bit off topic for this ticket, for anyone interested work in progress at:
https://github.com/hnla/hnla-bp-nav-menu

#7 @karmatosed
5 years ago

  • Cc karmatosed@… added

#8 @karmatosed
5 years ago

@hnla : maybe open a ticket for the menus as this could get confusing with them on here as not related and wouldn't be the scope of this ticket? I very much feel they need their own ticket as good stuff.

Just to be clear no changes are aimed at breaking. If anything they are to make it work better with themes. The only 'new' is being suggested colours and removing of some styling.

This is all going to be far better to be demonstrated in a plugin so that's what I'm starting to do here:
https://github.com/karmatosed/refreshdefault

It has nothing in it yet but the plan will be to (hopefully with others) work through the following list.

  1. Clean up
  2. Making sure fonts and font sizes come from theme (px to rem would also be good)
  3. Remove all border radius, gradients, drop shadows (if any)
  4. Messages (error, success, notices) : particularly checking contrast levels on this to make sure we are accessible
  5. Buttons (comment, reply, favourite) - ideally would use the theme .button style for colour and not have a gradient/radius
  6. Span number indicators
  7. Loaders

This will not be template changes. It will not substantially change the look. It won't HTML5ification all the things :P This isn't aimed at that. For that the other UI ticket would be the place :)
http://buddypress.trac.wordpress.org/ticket/4952

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

#9 @karmatosed
5 years ago

I've just added some new message colours for the error, update, and success messages on github: https://github.com/karmatosed/refreshdefault

#10 @hnla
5 years ago

I ran through and cleared up a few more little things but think you've attended to most of it.

Sadly I cloned form your account so can't issue pull request right now.

#11 @karmatosed
5 years ago

@hnla : cool that rocks. If you can do a pull request once you can and list what you've changed that would be great so can work to getting that in.

#12 @karmatosed
5 years ago

I've put a latest version on Github: https://github.com/karmatosed/refreshdefault. This now has the numbered table of contents to correct numbers and some small tidying.

I plan this week to explore removing all inner shadows and borders/backgrounds on form elements. However, have to see the impact of this. Not all themes declare them so it could look bad. It would make it easier for dark/different colour themes though.

#13 @mercime
5 years ago

  • Cc mercijavier@… added

#14 @karmatosed
5 years ago

A summary of the things that have been done for consideration to commit are:

  • Simpler button styling (uploaded to this ticket)
  • Removed borders (uploaded to this ticket)
  • Removed border radius (uploaded to this ticket)
  • Added new message colours with contrast check for accessibility (uploaded to this ticket)
  • Removed font px and forcing
  • Fixed comment order in CSS file
  • Removed comment numbers that had no entries

In general the changes are to 'detheme' the current version.

https://github.com/karmatosed/refreshdefault has the current set of changes

#15 @ubernaut
5 years ago

i like those :)

@karmatosed
5 years ago

#16 @karmatosed
5 years ago

I've put a patch up just so it's there if required as know the deadline is today and want to make sure everything's up.

#17 @hnla
5 years ago

Seeing this patch go up I was going to mention whether it had taken into account a recent commit by @boone ( https://buddypress.trac.wordpress.org/ticket/4116 ) adding in a series of input attr selectors to the sheets but checking this patch I see that those have been checkedout and removed in this patch.

I commented on the other ticket as there was request for 2nd-opinion on the commit and pointed to this this re-fresh ticket, but hadn't seen any updates to that ticket.

There is now the situation where tickets are pulling against each other at the moment, one has added stuff, the other removed it again.

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

#18 @karmatosed
5 years ago

I'd not consciously removed his changes. I have no issue with the selectors being added back in.

#19 @karmatosed
5 years ago

Latest patch takes into account the html5 elements.

#20 @karmatosed
5 years ago

  • Keywords has-patch added

#21 @boonebgorges
5 years ago

  • Keywords needs-testing added

Great work with this project, karmatosed et al.

I just spent a time testing out the changes with a handful of popular themes. Overall, the detheming has a very nice effect - it's great to see stuff like font faces and button styling being inherited from themes more elegantly. Makes it look less like bp-default.

The only obvious issue I found was with button text size. First, there was a line-height: 14px in there that looks like it should be removed. Second, by removing the font-size declaration, buttons are getting set to 1em. IMO, this is too big. The "Create a Group" button in the groups directory, for instance, ends up being enormous, because it inherits the <h1> font-size. Even the 'Join Group' buttons etc seem too large for me.

4953.patch removes the stray line-height, and sets button font-size to .8rem. I noticed that percentages were being used throughout the stylesheet for font-size, but since buttons appear in many contexts (as children of elements with different font-size values), using font-size: 80%; made the button text too large in some cases and too small in others. Thus, rem. Let me know what you think about that.

@boonebgorges
5 years ago

#22 @karmatosed
5 years ago

@boonebgorges: Thanks for taking a look and glad it's holding up - that rocks. Font size is kind of the 'bane of theme compatibility' so thanks for the catch on that. I'm happy with that patch but open to having an exploration for the impact of rem/other options.

Perhaps we need to check rems in IE though and at worst pop in some px fall backs for where we use them - easily done. Percentages as you rightly point out have a whole host of problems though themselves easily making very small text when not intended.

#23 @boonebgorges
5 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 7206:

Miscellaneous "detheming" of the bp-legacy stylesheet

When introduced in BP 1.7, the bp-legacy stylesheet was mostly copied over from
bp-default, with some of the more global declarations removed. However, much of
bp-default's personality could still be found in bp-legacy, in the form of
button styling, font families, rounded corners, and other such details.

This changeset removes some of these remnants of bp-default's styling, so that
sites using theme compatibility will inherit details more elegantly from the
active theme. The result is a much more consistent visual experience, with
buttons and other BP elements matching the rest of the theme more
seamlessly.

Fixes #4953

Props karmatosed, hnla

#24 @boonebgorges
5 years ago

Thanks for your amazing work on this one, gang. Let's handle cleanup in issue-specific tickets.

#25 @trishasalas
5 years ago

  • Cc trisha@… added

#26 @DJPaul
19 months ago

  • Component changed from General - UX/UI to Core
Note: See TracTickets for help on using tickets.