#6569 closed enhancement (fixed)
Let's give post-form.php the love
Reported by: | modemlooper | Owned by: | |
---|---|---|---|
Milestone: | 2.4 | Priority: | normal |
Severity: | normal | Version: | 1.2 |
Component: | Templates | Keywords: | has-patch |
Cc: |
Description
I think it's time we update post-form.php. The markup is ok, the styling can be enhanced but the action placements are a development nightmare. Look at attached image. The same button hooked to the only actions available. I've seen plugins override this post form template so they can add buttons in locations that are where they should be. This should not be the case. We should plot a few use cases and make it easier to get custom buttons/links/options into this form that is really the most used UI in all of BP and I think its the worst.
Attachments (11)
Change History (61)
#3
@
9 years ago
another issue is when you toggle form open and then try to select where to post is closes unless you have text in form
#4
follow-up:
↓ 5
@
9 years ago
Have you really not seen the umpteen number of tickets I've had and patches to try and address concerns for this aspect of the whats-new-form, this feels a bit like deja vu for me.
Write some patches, lets see some fixes, problem is though that changes will all to easily have back compat issues :(
#5
in reply to:
↑ 4
@
9 years ago
Replying to hnla:
Have you really not seen the umpteen number of tickets I've had and patches to try and address concerns for this aspect of the whats-new-form, this feels a bit like deja vu for me.
Write some patches, lets see some fixes, problem is though that changes will all to easily have back compat issues :(
not writing anything until it's something we want to improve but I constantly have to edit this file or override it.
#6
@
9 years ago
I think modemlooper is so right! But as hnla said, it's challenging !
imho: we should work on an extendable "full javascript" UI a bit like what we did with avatars with a fallback on the old post form if javascript is broken or unavailable.
This new UI could use a contentEditable div so that images can be displayed once we attach them to the new status update.
It would also be easier to bring our upload API in it without poping a thickbox for instance. Because, attaching photos requires being able to upload and to browse into user's already uploaded photos...
We also have the select box to choose the groups/profile thing that could be improve because it's not possible to add new choices... Well that's a really interesting challenge :)
If everyone agrees on the "full javascript" thing, i'd be happy to work on it ;)
#7
@
9 years ago
but I constantly have to edit this file or override it.
As do I but have attempted a lot of patches to address issues taking quite a bit of time, I revised some of the javascript but aspects of my revisions to aid developers were knocked back.
#8
follow-up:
↓ 9
@
9 years ago
I don't know about full JS, it would break existing plugins that add actions to markup. also so apprehensive about a media thing included in BP because almost every single client wants something different.
#9
in reply to:
↑ 8
@
9 years ago
Replying to modemlooper:
I don't know about full JS, it would break existing plugins that add actions to markup.
Yes, and that's one of the reasons it's so challenging to touch to any part of the post form, because some plugins are using existing actions. But one day or another, i'm pretty sure we'll need to go this road :)
In the meantime, i've made some tests with the post form template see 6569.patch.
- remove the
p.activity-greeting
and use it instead as the placeholder of thewhats-new
textarea - put the
div#whats-new-submit
out ofdiv#whats-new-options
- added a
div#whats-new-actions
that containsdiv#whats-new-submit
anddiv#whats-new-options
and some new actions. - edited some css rules and the main js file to auto-resize the textarea on keyup
In div#whats-new-actions
The main interesting action imho is the one before div#whats-new-submit
and div#whats-new-options
it's precisely bp_activity_post_form_before_options
. It seems logic to me that when posting an update we:
- write its content
- eventually attach media
- tell who is targeted
- submit the form.
Google+ post form is interesting, because it has a lot of "attachment" types (photo, links, video...) And it seems the more close to our case, because we want to make it possible to any plugin to add custom "attachment" types.
So i've played with this and the BuddyDrive plugin to see how a plugin could add its customs "attachment" types. Here's what you get when on the activity directory :
Only the placeholder is shown
When you begin to type a new activity, this is what you get (btns are generated by BuddyDrive by hooking on bp_activity_post_form_before_options
) :
When you click on the "Photo" button, this is what you get (Uploader is the BuddyPress one and generated by BuddyDrive)
also so apprehensive about a media thing included in BP because almost every single client wants something different.
I see what you mean, but on the other hand, some users could be frustrated by the fact there's no built-in component to manage user uploads in the long term. I agree there are some great plugins to do so, but i'm also hearing some users complaining when a plugin drop his support... I guess people could feel safer having a very basic component to organize user uploads, other plugins could take to the next level :)
#11
@
9 years ago
6569.02.patch brings some improvements such as making sure all custom inputs are also sent into $_POST
This ticket was mentioned in Slack in #buddypress by imath. View the logs.
9 years ago
#13
@
9 years ago
- Milestone changed from Awaiting Review to 2.4
The more i think to this, the more i think it's a required step before thinking to add any feature like 'attach a photo to an activity'. I think we should really work on this during 2.4. And about attaching a photo to an activity, i have a plan in case this ticket makes it into core :)
@hnla could you test the patch and eventually improve the markup/css part if needed ?
#15
follow-up:
↓ 16
@
9 years ago
- Keywords 2nd-opinion removed
- Version changed from 2.3.2 to 1.2
imath - I like the JS approach to grab all the form fields in 02.patch
and passing that to the AJAX post action.
We should separate the commits out in the following order: cosmetic fixes, new actions, JS AJAX post improvements.
#16
in reply to:
↑ 15
@
9 years ago
Replying to r-a-y:
We should separate the commits out in the following order: cosmetic fixes, new actions, JS AJAX post improvements.
Absolutely :)
#17
@
9 years ago
So, getting the extra form data in post submit is great but there is nothing you can do with it.
bp_activity_add() uses bp_parse_args and you can push meta to this but then the save args will strip off anything extra.
action hook bp_activity_add only passes you back the original args and not the activity object so you can't attach any of the data to the newly created item.
I propose we pass back the activity object , then we can get the ID and run any meta functions.
do_action( 'bp_activity_add', $r, $activity );
then we can do an easy hook to add the meta or process it however needed.
function try_add_meta( $r, $activity ) { bp_activity_update_meta( $activity->ID, 'key', $r['meta'] ); } add_action( 'bp_activity_add', 'try_add_meta', 10, 2 );
This ticket was mentioned in Slack in #buddypress by modemlooper. View the logs.
9 years ago
#19
@
9 years ago
'bp_activity_add'
is an example of a superfluous hook; BP has way too many of them. You should really be using the 'bp_activity_after_save'
hook for stuff like this.
With imath's patch, you should be able to check for your posted meta in the $_POST
global during the 'bp_activity_after_save'
hook.
#20
@
9 years ago
That hook is not in the patch, no extra data is sent. In bp_activity_add it explicitly selects data and nothing is passed to the after save hook. so no way to use anything from post global
#21
@
9 years ago
That hook is not in the patch
The 'bp_activity_after_save'
hook is not a new hook:
https://buddypress.trac.wordpress.org/browser/tags/2.3.2/src/bp-activity/classes/class-bp-activity-activity.php#L246
What I am saying is the 'bp_activity_add'
hook is not necessary when you have the 'bp_activity_after_save'
hook, which passes the full activity object when an activity item is saved into the database.
The following code snippet should work:
function my_activity_custom_post_save( $activity ) { if ( ! empty( $_POST['YOUR_FORM_VAR'] ) ) { bp_activity_update_meta( $activity->ID, 'key', $_POST['YOUR_FORM_VAR'] ); } } add_action( 'bp_activity_after_save', 'my_activity_custom_post_save' );
The reason why this works is imath's patch passes all the form fields from the activity post form during AJAX, which should allow you to check the $_POST
global for your form field value. You have to apply imath's patch first, then you can try the snippet above.
#22
@
9 years ago
I know the hook is there, never mind, its passing post. I had code elsewhere that interfered.
#23
@
9 years ago
We are gonna have some issues with post form and attachments depending on placement of UI elements. In 2012 the whats-new-options is a set height. I think we need some additional hooks
#25
@
9 years ago
- Keywords reporter-feedback added
@modemlooper have you tested the patch? It's including new hooks, re-organizing the post-form and making sure all fields will be included into the $_POST
global as explained in this previous comment
Is applying the patch is fixing the 'cut-off' issue ?
#26
@
9 years ago
So i've been working the BP Attachments plugin to check how this patch could be useful. I've found that when you're adding some extra fields to the post form, if the user tries to use them leaving the textarea empty, the whole form is shrinked which is terribly annoying. So i'm in favor of a reset button a bit like on the Google+ screencapture. The other advantage of this button is that plugins can listen to the reset event.
Here's a screen capture of the post form generated by the BP Attachments plugin (which basically uses the 6569.03.patch).
This ticket was mentioned in Slack in #buddypress by imath. View the logs.
9 years ago
#28
@
9 years ago
Hi, just a suggestion of change in the post form design. The question for me is always simplicity, how many times the user has to look around the post form to be sure that is posting in the right place and the right message, and also I know there are users thinking where they want to post even before starting typing.
That's why I believe that where to post option should be in the same eyeline and always visible from post typing area.
Also the avatar is quite big for me, maybe I'm wrong on this but we don't need to watch our pretty faces in big sizes all time, is quite useful to see where I need to start typing but not too much for bigger size than the writting line.
Just a couple of ideas.
#30
@
9 years ago
Linking the youtube video by imath here for posterity.
https://www.youtube.com/watch?t=5&v=yt5P6f9ojz0
#31
@
9 years ago
I'd like to request that any upload feature, ( profile photo, header, activity entry attachment, etc. ) support a check of image orientation and make necessary adjustment.
That data is now part of WP codebase.
https://core.trac.wordpress.org/ticket/28916
I can't be the only one with client complaints re 'sideways' images due to use of mobile devices.
#32
@
9 years ago
shanebp - Good point. I think your issue is the same as #5089, which is for avatars though.
This ticket is about helping plugins pass their data from the activity post form, so they can do stuff with it.
#33
@
9 years ago
04.patch
refreshes imath's awesome work from 03.patch
for trunk and includes a few cosmetic changes:
- Arrow is added beside the "What's New" textarea with CSS :before and :after. This might require tweaking for our companion stylesheets.
- Added new JS - autoresize.js - from Jack Moore (Licensed under the MIT.) This is used to resize the textarea with no jittery bugs and also handles resizing the textarea during text deletion.
- Tweaks JS to remove the
'active'
CSS class onfocusout
. Before the'active'
class was always there after the initial focus.
#34
@
9 years ago
Questions from the peanut gallery:
- Is autosize.js something we might use somewhere else? Should we have it as a core script rather than in the theme compatibility folder?
- If you click in the box / enter text, and then delete it all, and click out the box, the "Post in" "cancel" "update" buttons etc are still shown (these only appear when you click in the box initially)
This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.
9 years ago
#36
@
9 years ago
I tried .04 in the most recent Firefox, Chrome and Safari with success. These are nice improvements, and even just moving the js and form element improvements in for 2.4 would be great.
#37
@
9 years ago
@r-a-y your gif is awesome. I'm sorry i haven't had time to test your patch, but it sure looks great :)
My only fear, since we probably won't progress on giving a tool to users to help them figure out if their templates are out of date, is potential back compatibility issues.
This fear is telling me to work on this later. But if you feel confident about it and think my fear is not justified, let's do it :)
Thanks a lot for your great work about this ticket.
#39
@
9 years ago
Good point about the template changes, imath!
I've created a new patch - js-only.patch
- that does the following:
- Adds imath's improvements to pass all valid input fields from the "What's new" post form to our AJAX post call. This will allow plugins to save their data.
- Removes all our weird custom
animate()
calls to useslideUp()
/slideDown()
. This fixes modemlooper's problem with buttons being truncated in comment:23. - Tweaks JS to remove the
'active'
CSS class onfocusout
as explained in comment:33. - Fixes issues when selecting an option in the "What's New" form as described in the GIF by modemlooper in comment:2.
- Uses
autosize.js
as explained in comment:33.
No changes to our templates. Template changes should probably be done in v2.5.
A few notes about autosize.js
:
- It does not support IE8.
- There is some jumpiness in IE10-11; I've added a temporary CSS hack to fix this, which adds
padding-bottom: 22px
to the<textarea>
element. I know we hate CSS hacks, so this probably will be omitted. Or we might even omitautosize.js
from committing.
#40
@
9 years ago
@r-a-y Just tested the patch, thanks a gain for this great work.
I think i'd follow you on this for 2.4 :
Or we might even omit autosize.js from committing.
Something is still wrong:
Using twentyfifteen, when i post an update, the form is not coming back to its initial state:
^^
Once updated - this wouldn't be to bad if we could avoid the next behavior
^^
On focus, we don't have full height of the textarea row, when typing the text is truncated at the bottom.
Finally, when you focus out having text inside the textarea, the post update button disappears.
I think we need more time to fix this very annoying behavior with the post form.
I'm sorry r-a-y, this might be a bit frustrating but even if you improved the behavior of the post form with your 2 latest patches, i think i would only include the fix to make sure extra inputs are added to posted data for 2.4.
#41
@
9 years ago
Thanks for testing, imath.
js-only.02.patch
should correct all the issues that you listed in comment:40.
In that patch, I'm saving the initial height of the textarea and using that to set the height after AJAX posting. Also, the form doesn't slide up any more if there is content in the textarea.
Let me know if you spot any other issues!
Update: Removed unnecessary styles in companion stylesheets since JS now covers the #whats-new-options
block better.
#42
@
9 years ago
looks ok, but why do we add a button to cancel rather than simply close up onblur if text area empty.
Styles will need addressing for the submits at small screen, 2015 is a bit squashed together.
The arrow pseudo element affect doesn't completely disappear at smaller screen there's a point where the border overlap is still present causing a broken looking border.
#43
@
9 years ago
Thanks for the feedback, hnla.
looks ok, but why do we add a button to cancel rather than simply close up onblur if text area empty.
.
Styles will need addressing for the submits at small screen, 2015 is a bit squashed together.
We're going to move the template changes for v2.5 so we can talk more about things like the "Cancel" button and so we can refine the experience a bit more.
The arrow pseudo element affect doesn't completely disappear at smaller screen there's a point where the border overlap is still present causing a broken looking border.
True, I did not do any responsive work on that yet. The CSS arrow also isn't in the js-only
patches.
#44
@
9 years ago
I tried 6569.s-only.02.patch on my test install in recent Safari, Firefox and Chrome, and thought the js work improves the end-user experience and is very useful for laying the groundwork for future extensibility.
Thanks, @r-a-y, for working on the activity form. This is a really important bit of the user interface.
This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.
9 years ago
This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.
9 years ago
#49
@
9 years ago
- Keywords reporter-feedback removed
- Resolution set to fixed
- Status changed from new to closed
Going to close this one for v2.4.0.
For the activity post form template changes, I've opened a new ticket - #6680 - for v2.5.0.
Let's continue this discussion over there. Thanks everyone who has commented and taken part in the work for this ticket so far!
twitter post form