Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mark talk as Watched/Unwatched #383

Merged
merged 8 commits into from
Nov 20, 2024
Merged

Conversation

nicogaldamez
Copy link
Contributor

Why?

What?

  • I added a button below the video player to mark the talk as watched or unwatched
  • The button is only visible to signed-in users.
image image

@nicogaldamez
Copy link
Contributor Author

@marcoroth @adrienpoly any idea why CI is failing?

image

@adrienpoly
Copy link
Owner

This is the test that run the entire seed, it only runs in the CI as it is fairly long this is why you probably don't see this error in dev.

To go back to the error by itself I had a similar case in the past. I think we might have to either remove the constraint or make it deferrable

More info here :
rails/rails#49376

@nicogaldamez
Copy link
Contributor Author

Thanks, @adrienpoly ! I had to remove the constraint because making it deferrable didn't work.

@adrienpoly adrienpoly force-pushed the main branch 2 times, most recently from ca98c66 to aa6f659 Compare November 10, 2024 15:03
Copy link
Collaborator

@marcoroth marcoroth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thanks @nicogaldamez! 🙌🏼

I think I only have one minor thing about the icon for the "unwatch" action, but other than that this looks great to me!

app/views/talks/_talk.html.erb Outdated Show resolved Hide resolved
@adrienpoly
Copy link
Owner

I like the way it is shaping, good job!

I ran this PR locally an I am getting this error when I click a second time the button, not sure if there is something with my db or what???

CleanShot 2024-11-14 at 21 59 28@2x

@nicogaldamez
Copy link
Contributor Author

@adrienpoly that shouldn't happen. When you mark a talk as watched, the button should change to "Unwatched", and it shouldn't try to mark it as watched again. Anyway, I replaced the create! with a find_or_create_by! to avoid this issue.

Let me know if you still have any problems.

@marcoroth
Copy link
Collaborator

Thanks for working on this @nicogaldamez. I slightly tweaked the styles of the button. Before I was slightly confused in which state a video is. I hope this improves the usability aspect a bit and hope you don't mind. 🙌🏼

CleanShot 2024-11-20 at 02 51 47

@marcoroth marcoroth merged commit bf3eee2 into adrienpoly:main Nov 20, 2024
4 of 5 checks passed
@nicogaldamez
Copy link
Contributor Author

@marcoroth I love it 🤩

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants