#5918 closed enhancement (fixed)
Add a CSS class to dynamically generated warning messages
Reported by: | SGr33n | Owned by: | 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)
Change History (16)
#1
@
10 years ago
- Keywords needs-patch good-first-bug added
- Milestone changed from Awaiting Review to Future Release
#2
@
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
@
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
@
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
@
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 :)
#7
@
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.
#8
@
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?
#9
@
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:
↓ 11
@
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!
#12
@
10 years ago
- Owner set to boonebgorges
- Resolution set to fixed
- Status changed from new to closed
In 9058:
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.