mattstauffer / suggestive

Suggestive - take suggestions from the fans/followers of your podcast or blog
MIT License
98 stars 26 forks source link

Implement topic comments #17

Closed mrterryh closed 7 years ago

mrterryh commented 7 years ago

I've been wanting to get into open-source for a while now, so this is my first real PR to an open-source project! I've implemented a basic commenting feature and in turn, a single topic view component. Oh, and I included tests!

I've tried my best to keep my coding style inline with that of the current project. Apologies if I missed anything!

I know you like GIFs, so here ya go:

screen recording 2016-10-18 at 07 54 pm

mattstauffer commented 7 years ago

WHOAA I AM EXCITED!!!! 🙌 Gonna check this out shortly--thank you!!

mrterryh commented 7 years ago

No problem, had a blast with this!

mrterryh commented 7 years ago

Ok, I think I've addressed all of your points. Let me know if I've missed anything!

mattstauffer commented 7 years ago

This is super fun to work with.

We're having a bit of Moment.js trouble here:

image

The time zone is set to UTC, and I'm in EDT, so that must account for the difference. What will it take to get Moment.js to detect the user's time zone? Is that easy or awful?

mattstauffer commented 7 years ago

Additionally, and this can be a later PR if this is overwhelming, we'll want the admins to be able to see the comment threads:

image

Apparently they have a different view to show their topics-in-list since they have the buttons, so your changes to make the topic title linkable didn't push through there.

mattstauffer commented 7 years ago

Finally: I made a few changes locally and pushed to the mrterryh-implement_topic_comments branch (https://github.com/mattstauffer/suggestive/tree/mrterryh-implement_topic_comments). Can you pull those changes down into your fork?

Thanks! let me know if you have any issues with that!

mrterryh commented 7 years ago

Additionally, and this can be a later PR if this is overwhelming, we'll want the admins to be able to see the comment threads:

I'll do this in a separate PR if you don't mind! I'll separate that topic.vue into three components. Maybe something like: topic.vue, comments.vue and comment-form.vue

As for the moment thing, I don't have much experience with timezones (luckily), but I've done a bit of researching on StackOverflow and think I may have solved it. It's displaying correctly for me though, so I can't test it! Kinda hard to debug haha!

mattstauffer commented 7 years ago

Sounds great! Looks like the fix was close to what you wrote:

+ comment.created_at = moment(comment.created_at.date).local().fromNow();
- comment.created_at = moment.utc(comment.created_at.date).fromNow();

Thanks for all your hard work on this. This is really great!