Skip to:
Content

BuddyPress.org

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#5918 closed enhancement (fixed)

Add a CSS class to dynamically generated warning messages

Reported by: sgr33n's profile SGr33n Owned by: boonebgorges's profile boonebgorges
Milestone: 2.2 Priority: normal
Severity: normal Version:
Component: Core Keywords: has-patch
Cc:

Description

Hi,

I see that BuddyPress gives to many divs the #message id. We can subdivide them in two categories: the one added via Ajax (like errors or confimations) and the one static (like when you're on the messages page and there are no messages). I would suggest to give a different class to the first and to the second type, in order to style them differently. E.g. in a theme could happen that we'd like to display with "absolute" the first type, but it could affect also the second "static" type and differentiate them via css could be difficoult.

Attachments (2)

confirmation-messages.patch (9.3 KB) - added by SGr33n 10 years ago.
Sliding down confirmation messages
5918.patch (3.8 KB) - added by boonebgorges 10 years ago.

Download all attachments as: .zip

Change History (16)

#1 @boonebgorges
10 years ago

  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to Future Release

Hm, interesting. What's the use case for this? Maybe you're hoping to use CSS3 transitions with the AJAX ones or something?

I don't have a problem adding a class (in particular, to the dynamically created them). If we get a patch, we can put this into a milestone.

#2 @SGr33n
10 years ago

Hi Boone,

Really good, thanks :) Yes, I'm using CSS transitions/jquery to slide down all dynamically created messages from the top of the window. Anyway on the current BP Legacy Theme #message will appear in various positions, so I have to remove them from the default positions and append to the body via JavaScript, the result is very nice :)

#3 @boonebgorges
10 years ago

That sounds great. Please do feel free to provide a patch with how you think this might work, and we can fit it into an upcoming release.

#4 @SGr33n
10 years ago

I'd love to! I try to split the whole #message code from the theme I'm working on and I'll attach the patch files here. So don't care if the "No messages" div will messed up?

#5 @boonebgorges
10 years ago

So don't care if the "No messages" div will messed up?

We can talk about that once we've seen the patch :)

#6 @SGr33n
10 years ago

lol :P good :) so see you here soon

@SGr33n
10 years ago

Sliding down confirmation messages

#7 @SGr33n
10 years ago

This is a start... the style looks like the Default WP one, with the white background and a 4px left border green for success and red for errors.

This will show up when e.g. you try to post an empty comment.

Last edited 10 years ago by SGr33n (previous) (diff)

#8 @boonebgorges
10 years ago

Gotcha. That looks pretty neat. Are you proposing this specific UI change for BP, or just demonstrating what you'd do if you had the extra classes?

Check out 5918.patch. It adds the 'bp-ajax-message' class to #message divs returned via AJAX. Is this what you had in mind?

@boonebgorges
10 years ago

#9 @SGr33n
10 years ago

If you agree absolutely proposing :) Anyway these are not the only messages we can show in that way, there are also messages shown on the next page, when the form is not handled via Ajax (like when editing profile fields - displayed in the item-header - ).

This second type is handled on my theme.js removing the #message from its container and appending it to the body (otherwise it could not be shown at that dimension - percentage of the full window - and in that position).

	/* Manage error messages */
	if ( jq('div#message').length) {
		jq('div#message').appendTo( 'body' );

That part is missing in the uploaded patch file.

Actually if you can merge this into the bp-legacy you could also consider to directly append these messages to the body, so we can skip the html manuplation part at all, or, at least, can show them in a standard place (e.g. after #buddypress).

#10 follow-up: @boonebgorges
10 years ago

  • Keywords has-patch added; needs-patch good-first-bug removed
  • Milestone changed from Future Release to 2.2
  • Summary changed from About warning messages id/classes to Add a CSS class to dynamically generated warning messages

We'd need to do quite a bit more work to make your proposed changes work across themes. For example, we'd need a more theme-independent way to determine the vertical offset.

More broadly, moving some messages to JS-powered flyovers creates some fracturing of our UX - these messages are no longer in keeping with our non-AJAX-powered messages. I think that we probably can't do this in bp-legacy, for backward compatibility reasons. Perhaps as part of a new template pack? See https://github.com/karmatosed/buddypress-templates

But yes, you are right about funky DOM manipulation - using overlays for this stuff would simplify that part of our error messaging.

I don't have a problem adding the requested CSS classes to bp-legacy for 2.2. Can you verify that 5918.patch will fit the bill? If you would like to make a more formal proposal for the fancier changes, please open a new ticket and try to provide a cleaned-up patch - your previous patch had a couple other miscellaneous fixes.

Thanks!

#11 in reply to: ↑ 10 @SGr33n
10 years ago

Well! :) I tried 5918.patch, it's working good.

Thanks!

#12 @boonebgorges
10 years ago

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

In 9058:

Add 'bp-ajax-message' class to message divs returned in AJAX responses.

This allows for better styling of these dynamically created items.

Fixes #5918.

#13 @boonebgorges
10 years ago

Thanks for testing, SGr33n! If you'd like to propose something broader regarding your message styling/behavior, please open a new ticket.

#14 @SGr33n
10 years ago

Sure! thanks :)

Note: See TracTickets for help on using tickets.