Preview posts in stream #4099
Preview posts in stream #4099
Conversation
|
Wow, awesome! Users would totally love this! On Thursday, April 4, 2013, svbergerem wrote:
|
|
Great work! Code wise I think you're on the right path, did you think about creating a new UI wise I think that's okay for a first iteration, but maybe our UI gurus have some ideas already. I'm attaching a screenie for convenience ;) |
|
@MrZyx Yes, I already tried to create a new I am also looking forward to see some suggestions from the "UI gurus" :) |
|
Mmmh, |
|
Ok, then just tell me what var statusMessage=new app.models.Post({
"status_message" : {
"text" : serializedForm["status_message[text]"]
},
"aspect_ids" : serializedForm["aspect_ids[]"],
"photos" : serializedForm["photos[]"],
"services" : serializedForm["services[]"]
});and to add the object to the stream I did |
|
Hm, indeed, looks like we're just abusing the bacbkone model to construct the AJAX call, the data structure is completely different between submit and receive, wasn't aware of that. Since this won't change without larger refactors on the Rails side too, your approach looks good as a intermediary solution :) You could pass the object you're currently building in |
|
I added a preview for photos and wrote a cucumber feature. I don't think that adding mentions and oEmbed will work without blowing up the code and I also don't think that they are that important for a preview. IMO we should add them after the possible refactors @MrZyx mentioned. |
|
If we do a preview, I think it's important that the preview looks exactly like the real post. |
|
@Flaburgan Ok, I added mentions in the preview. At the moment the link to the userpage is broken because I need the guid for that but just got the name and the diaspora-handle. After sharing a post oEmbed doesn't work in the stream either. You have to reload the stream to see the content. If that has to be in the preview it also has to be in the stream after sharing the post. (and I don't know how to do that) Edit: Fixed the link to the userpage. |
| var regexp = new RegExp("@{\(\.\*\) ; \(\.\*\)}", "g"); | ||
| while(user=regexp.exec(serializedForm["status_message[text]"])){ | ||
| // user[1]: name, user[2]: handle | ||
| var mentioned_user = Mentions.contacts.filter(function(item) { return item.handle.toLowerCase().indexOf(user[2].toLowerCase()) > -1 })[0]; |
There was a problem hiding this comment.
I found that in /app/assets/javascripts/mentions.js. Is there another (shorter) way to get the JSON of the user?
There was a problem hiding this comment.
the easiest way would probably be to write a helper function and use that in both places. (and to write a jasmine test for that new helper function)
;)
There was a problem hiding this comment.
What exactly should the helper function do? I copied the line from mentions.js but then I changed a few things because here I want to get the JSON of a single user with his handle, in mentions.js we need an array of users with a name including a given string.
Edit: I just changed the line because we look for a contact with the same handle, so I now use item.handle == user[2]
|
About the backbone.js model ... I'd have to take a closer look, but I guess the way it is handled now is just as good as any other, Unfortunately the backbone models are not entirely the same when an object is created client side vs. the server side... that's probably an artifact from the old pre-backbone UI code. If we ever get to do a proper API, we should start with a separate backbone-UI JSON API... |
|
Wow, awesome! This has to be the single most wanted feature :) |
|
|
||
| .post_preview | ||
| :border | ||
| :bottom 3px solid #3f8fba !important |
There was a problem hiding this comment.
maybe also let it have a very light blue or gray background color, so that it really looks different from the rest of the posts in the stream.
|
Just pulled on my pod and works great - such an awesome feature, people are going to love it! Also the light blue background is a good addition to distinguish it from the stream. Nice one! |
|
Another screenshot with the light blue background: |
|
Great! |
|
Is there anything that needs to be done before this pull request is ready to merge? |
|
Less of a bug, more a behaviour decision: Currently the preview button does nothing when just uploading some photos without entering any text. I'd also remove the comments inside the JSON constructions and I'm not sure why you changed the URL for the exisiting changelog entry, especially since the new one just redirects to the old :) |
|
Oh and maybe we should find a way to disable/remove the comment/like buttons in the preview. If there's no easy one I'd be fine pulling this in as it is as a first iteration though. |
|
Allright the classes of the comment/like buttons are now changed in the preview so they don't do anything. I removed the comments in the JSON and changed back the URL in the changelog. The behaviour you described is weird. On textchange Edit: I just found out that uploading a photo and then deleting it gives you the opportunity to share an empty post. So the share button just gets enabled on changes in the photodropzone but not disabled again. |
|
Did you try to add embed support? Just look at oEmbed i don't think this is hard to do. |
|
@Flaburgan it is and out of scope of this PR. @svbergerem I fear it's legacy hackery :/ https://github.com/diaspora/diaspora/blob/develop/app/views/photos/_new_photo.haml#L32 |
|
@Flaburgan No, I did not add oEmbed support. There is an issue because of missing oEmbed support when sharing a post and I thing this should be done in a seperate PR for both. |
|
Okay, we can merge that and deal with oembed after, but I think we should improve that. |
|
@MrZyx Ok, I added the same legacy hackery for the preview button, so both buttons should be treated the same way now. |
| // (e.g. bookmarklet, mentions popup) | ||
| if( this.options.standalone ) { | ||
| this.$('#hide_publisher').hide(); | ||
| this.$('.post_preview_button').hide(); |
There was a problem hiding this comment.
you can now use this.el_preview.hide() here...
There was a problem hiding this comment.
yep, changed the line.
|
Ok, one last little comment from me, and then this is good to go IMHO. In a subsequent iteration, the way we extract the form data from the publisher for posting and preview should really be unified to get more DRY. Also, anybody who wants to Backbone-ify the image uploading code is very welcome to do so (I will gladly assist anyone who is brave enough to attempt this) ;) |
|
@Raven24 What exactly do you have in mind? I have the var |
|
most of the format problems are in the user defined markdown, the object embed is either valid or not this be very handy! :) |
|
@svbergerem I meant that should happen at a later refactoring. I haven't really thought about anything specific ;) |
|
@Raven24 Ok, now I understand what you are talking about :) By the way the Travis build fails but IMO that has nothing to do with this PR. |
|
merged. |
|
Thanks to all who were involved in this. You help new developers a lot! |
|
Thank you very much for your work, this was a really waited feature! |
|
testing this on my pod, it is beyond wonderful! many thanks. :) one small problem i note is: if you are in a tag stream (from searching on a tag), it does not pick up on the content and just previews "The think about tag is...." but if find no other problem i any other places :) |
|
I think that has nothing to do with the preview. The publisher on the tag stream is a known problem #4017, so if you share your post instead of previewing it the text should be the same (The thing about #tag is...). If it does not just tell me. :) |
|
that is very possible. i did not post it. :) i'll report back here if that does not happen :) thanks again for the great feature. :) |
|
Just wanted to say huge thanks to @svbergerem and everyone else who helped for this great feature - it answers the request from thousands of people to have previews. I've just noticed a possible bug with it: I went to a tag page (#cricket) to test out the fix to the bug in posting from tag pages ('The thing about #tagname is...') and when I pressed the Preview button, instead of the post preview having a blue background, the name of the first tag follower in the list on the left went blue. It's only a very minor thing, and didn't stop the preview from working at all, but you might want to look into it. Thanks. |
|
@goobertron sure, would you mind opening a separate report about this? |
|
Yes, of course. Sorry, I didn't know the form. |
#2108
This will add a preview button to the publisher. When you press the button the current post will be presented in the stream. When you share the post the preview will disappear.
Just like the share button the preview button is disabled if there is nothing to post.
Currently not working: