Skip to:
Content

Opened 7 months ago

Closed 5 months ago

#5167 closed defect (bug) (fixed)

CSS breaks wp_editor on BP pages

Reported by: netweblogic Owned by: boonebgorges
Milestone: 1.9 Priority: normal
Severity: normal Version: 1.8.1
Component: Core Keywords: ui-feedback has-patch needs-testing
Cc: netweblogic, karmatosed, hnla

Description

I discovered this problem in my plugin Events Manager, but this is not related to EM rather CSS that is applied to BP pages.

What happens is the toolbar and add media buttons are bigger than they should be, which is caused by some rules in the BP stylesheet.

This is happening on TwentyTwelve

Proof of concept:

Visit the settings > general page in your profile (where you set your email and password) with this hook active:

add_action( 'bp_before_member_settings_template', 'wp_editor_bug_proof' );
function wp_editor_bug_proof(){
	wp_editor('test','test', array() );
}

Attachments (2)

5167.patch (3.1 KB) - added by boonebgorges 5 months ago.
5167-correct-comments.patch (488 bytes) - added by hnla 5 months ago.

Download all attachments as: .zip

Change History (12)

comment:1 follow-up: boonebgorges7 months ago

  • Cc karmatosed added
  • Keywords ui-feedback added
  • Milestone changed from Awaiting Review to 1.9

Confirmed. This looks to be caused primarily by a couple of padding declarations:

Removing the padding declarations pretty much clears up the issue, but it's not clear what we should do in BP to fix it. karmatosed, is there any reason why we can't just remove the padding, as part of ongoing detheming?

netweblogic, you can override those specific styles in your plugin if you want to work around the issue in the meantime.

comment:2 rogercoathup7 months ago

We hit the same problem -- the BuddyPress CSS is too generic here, it's targeting all tables inside the #buddypress selector.

The BuddyPress selectors should be made more specific - just targeting the tables they are supposed to be styling -- messages, notification, and profile

comment:3 in reply to: ↑ 1 netweblogic7 months ago

Replying to boonebgorges:

netweblogic, you can override those specific styles in your plugin if you want to work around the issue in the meantime.

Boone, thanks for your input, it helped.

Whilst I'm sure the offending lines of CSS you referred to has other implications for any plugin making use of tables on BP pages (including EM in other parts, but nothing major thankfully), here's a couple of lines that clean up the editor so it's useable:

#buddypress .wp-editor-wrap table { 
	width:auto;
}
#buddypress .wp-editor-wrap table tr td, 
#buddypress .wp-editor-wrap table tr th { 
	padding:0;
}
#buddypress .wp-editor-wrap a.button,
#buddypress .wp-editor-wrap button,
#buddypress .wp-editor-wrap input[type=submit],
#buddypress .wp-editor-wrap input[type=button],
#buddypress .wp-editor-wrap input[type=reset] { 
	padding: 0px 10px 1px;
}

This sorts out padding, the only other probem I notice are the media and html buttons are shaded slightly differently according to your CSS, but it's very subtle and acceptable!

comment:4 boonebgorges5 months ago

  • Cc hnla added

netweblogic - Thanks for sharing these additional details.

karmatosed, hnla, etc - I'm thinking of adding a bp-table class to tables that are produced by BuddyPress templates, and then applying our blanket #buddypress table customizations only to #buddypress table.bp-table. This seems to me more sane than listing all of the 6-8 places in BP where we use tables, and need to style them. The downside is that it'd mean changing templates.

I'm going to leave this in the 1.9 milestone. It's an easy change, but I do want a bit of feedback from someone from our template team before moving forward.

comment:5 hnla5 months ago

Adding '.bp-table' as a class for tables would have been the preferred state, however looking through the templates - at least in respect of 'messages', 'profile', & 'notifications' - we do actually have a mixed bunch of tokens present so I think my preferred option would be to adjust what ever rules are causing the issues ?'padding' to be grouped rules targeting those available tokens, even though this isn't ideal for the small number that we would have to do this for probably the better option from the point of view of upsetting established overloaded theme templates (of course either way could be an issue, the styles could have been overloaded entirely, but in that case we assume the user knows what they are doing and will establish their own adjustments)

This is a half a dozen on one scenario though, personally I prefer not to see templates adjusted even though it's the sane solution, obviously moving forward with e.g temp pack it's not an issue.

Alternative is remove those rules as in some respects we shouldn't have added properties for tables which as a construct handles fundamentally important properties it's self, it feels as though we were attempting to compensate for rules elsewhere styling tables and thus perhaps falling foul of attempting to correct for a given theme or themes.

comment:6 boonebgorges5 months ago

  • Keywords has-patch needs-testing added

Thanks very much for the feedback, hnla. You've convinced me that, on balance, it's probably best to just change the styles and leave the templates alone. Let's be sure to fix this problem the right way in the new template pack.

5167.patch is my attempt at solving the problem stated here. It does two things:

1) Removes all general style declarations for #buddypress table in favor of table-class-specific declarations. I *think* I have caught them all; please double check.
2) The editor style buttons can't be fixed in the same way - we just happen to have class name overlap. Because embedding a wp_editor() within BP content is probably a fairly common use case, I think we're justified in including a few declarations that are specific to the editor (.wp-editor-wrap), to fix the button styles.

Would like to get feedback from someone (esp hnla) before committing to make sure this seems like a sane approach for 1.9.

boonebgorges5 months ago

comment:7 hnla5 months ago

Ok patched the css, and added a wp_editor() instance to a plugins.php screen to check that.

wp_editor now looks correct, padding not breaking the layout.

Other bp tables look to be ok, I see a slight issue with right buttons and padding on messages tables but not 100% sure where that's coming from, needs checking further.

it's not the most elegant solution, but think it's best for the moment, moving forward the right way is a .bp-table class which we can add for template packs.

I would suggest we commit this to beta and I'll check it through again and see where the button issue lies if it is indeed an issue and not something I've introduced.

p.s there is one little '0px' that could be taken back to '0'

comment:8 boonebgorges5 months ago

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

In 7595:

Use more specific selectors in bp-legacy table CSS

The previous table styles were too general, which caused styles to leak
through inappropriately to non-BP tables. This was a particular problem for
the table-based syntax of rich text editors embedded via wp_editor().

Fixes #5167

Props netweblogic, hnla

comment:9 hnla5 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Very minor correction to C style comment to change to CSS syntax.

comment:10 boonebgorges5 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 7604:

Correct CSS comment style.

Fixes #5167

Props hnla

Note: See TracTickets for help on using tickets.