Captions

[gitriley]

Caption editing should work from the edit-title-card view, data-detail view, and the spreadsheet.

PHOTO

VIDEO

AUDIO

As you can see, I display the photo and video when such an item is being edited, but not for audio. It just seemed like a bit of extra work to get an audio player worker in there, and I wasn't sure if it was really necessary. However, if you want the audio player too, I can get it working in there, no problem.

One other thing: Right now, the editModel() function is created separately in the edit-title-card view, data-detail view, and the spreadsheet, and passed into the EditMediaInfoView each time. Since it's the same function every time, I think we can move it into the EditMediaInfoView, right? I haven't done this yet, but let me know what you think.

Also created some tests for the EditMediaInfoView.

[gitriley]

I went ahead and moved the editModel() function into audio-player and photo-video-viewer, rather than passing them in, in commit 19741627c67015f4dd1fccccb42ee0b45a75319e. If you don't want this commit, we can move back to the previous commit.

[vanwars]

Hi Riley,

Nice work. Some quick functionality notes:

Photos & Video:

• Editor looks good. Can we ask @cynthiamah what she thinks about auth + name + caption? It's nice to be able to have metadata for the photos for the data API, but it might be overkill for the UI. Not sure.
• For carousel, can we put captions above dots?

Audio:

• Editor: can we have an audio preview?
• Player: formatting TODO (for future PR or for this one): Alignment a bit off.

Regarding your editModel questions: sounds fine to me!

[gitriley]

• Ok, I can remove auth + name if we just want caption, but I'll wait on Cynthia's word.

• For the dots and caption placement, are you referring to the normal photo carousel that we use throughout the app? I think I remember us having a specific decision to move the captions below the dots about a year ago. I can't remember why, but I know we decided it after some deliberation. Do you remember?

• Ok, I will add the audio player.

• Audio player formatting: Are you just referring to the placement of play button? I can fix that, but let me know if you're referring to something else so I know what to fix.

[vanwars]

Carousel Formatting

@cynthiamah @gitriley : What do you think of deprecating the dots and doing a X of Y in the upper left-hand corner (like screenshot below)?

Audio Player Formatting

Yes -- just placement of the play button

[gitriley]

X of Y in the upper left-hand corner: I like it, and it's definitely seems preferable if a user has ~10+ images. But I also don't have any problem with the dots, so I guess I don't feel too strongly either way.

[gitriley]

@vanwars Sarah, for some reason the last Travis build failed after my last commit. It looks like one backend test failed, which is confusing because I don't think any backend files have changed in this branch.

Also (possibly related): I was getting the error 'This relationship already exists in the system' when trying to add videos to the data-detail today.

Confused.

[vanwars]

Hey Riley,

I already took a look -- for some reason, the 'rotate photo right' and 'rotate photo left' intermittently fail on Travis (but not locally). Just ignore it -- not you.

Re: "Also (possibly related): I was getting the error 'This relationship already exists in the system' when trying to add videos to the data-detail today." -- that's just the server telling you that you can't add a video to the record because that video already belongs to the record. Is it possible something is posting twice?

On Tue, Aug 28, 2018 at 9:49 PM Riley Flynn notifications@github.com wrote:

@vanwars https://github.com/vanwars Sarah, for some reason the last Travis build failed after my last commit. It looks like one backend test failed, which is confusing because I don't think any backend files have changed in this branch.

Also (possibly related): I was getting the error 'This relationship already exists in the system' when trying to add videos to the data-detail today.

Confused.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/LocalGround/localground/pull/221#issuecomment-416806168, or mute the thread https://github.com/notifications/unsubscribe-auth/ABKGw9pTU1ikeqhgBNAyCtGfOeDsKK_Iks5uVgFVgaJpZM4WKXoL .

[vanwars]

Hey @gitr,

I already took a look -- for some reason, the 'rotate photo right' and 'rotate photo left' intermittently fail on Travis (but not locally). Just ignore it -- not you.

"Also (possibly related): I was getting the error 'This relationship already exists in the system' when trying to add videos to the data-detail today."

That's just the server telling you that you can't add a video to the record because that video already belongs to the record. Is it possible something is posting twice?

[gitriley]

"That's just the server telling you that you can't add a video to the record because that video already belongs to the record. Is it possible something is posting twice?"

What's weird is it's happening even when the record doesn't have anything attached to it at all. Here's the network panel.

[vanwars]

Two final enhancements needed:

1. remove name from video and photo form
2. for full screen carousel, add author if it exists in the system

[gitriley]

@vanwars This branch is complete now

[vanwars]

Looks great! Merging.