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

[#8} For you page #24

Merged
merged 14 commits into from
Jul 26, 2024
Merged

[#8} For you page #24

merged 14 commits into from
Jul 26, 2024

Conversation

paulmckissock
Copy link
Contributor

[#8] Added the for you page. Users can go back and forward in a randomly sorted list of all sounds which is stored in the session. There is an autoplay checkbox that, when checked, plays the next sound as soon as the current one ends. Also setup black.

Copy link

@noahko96 noahko96 left a comment

Choose a reason for hiding this comment

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

Mentioned a few things below. I would also get CI stuff set up to run your tests. Also, for future reference, when adding a formatter to the code, do that in a separate PR so the new stuff is distinct from the format changes, makes it easier for review.

<p>No audio files available.</p>
{% endif %}

<script>

Choose a reason for hiding this comment

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

Did you or ChatGPT write this JS code? If ChatGPT wrote it, I think it would be a good exercise to go through it and describe what it is doing in detail in a comment. I think that would be beneficial because it is important that you understand this code in case ChatGPT is not 100% right and also this is meant to be a learning experience and using ChatGPT is not really learning in my opinion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At first, I tried to use ChatGPT to do it all at once but so many things broke and debugging was a nightmare because I didn't understand how anything worked. After that, I restarted and went through and added each part of the page one by one and debugged until it worked. This took me a while and I used ChatGPT to debug but I feel like by the time I got likes working and updating I had a pretty decent understanding of what was going on.

Description:

getAudio function:
This is called every time you need to use a GET method. It talks to the server and updates everything on the page. The action param is used in the url so it knows which get method it should request. The line xhr.setRequestHeader('X-Requested-With', 'XMLHttpRequest'); sets the header which is later used in my views so it knows to give a JsonResponse with updated data. If the get method was successful it goes through and updates every element with corresponding data that came from the xhr response. It then disables the next or previous buttons if they are at the beginning or end. Additionally, It makes the audio player play if the autoplay box is checked. For comments, it deletes everything marked comments list and makes another comments list using the comments from xhr response.

postAudio:
It basically does the same thing as getAudio but for POST methods. The main difference is that a formData param is passed which is sent here:xhr.send(formData); I was thinking about combing getAudio and postAudio but didn't get to it today.

After that, there are a bunch of event listeners that call getAudio or postAudio with their necessary params.

The autoplay-checkbox event listener is the only one that doesn't call postAudio or getAudio because no fields on the screen need to be updated. It sends get update_autoplay which is a method that saves the current state of the checkbox in the session.

audioapp/views.py Show resolved Hide resolved
Copy link

@noahko96 noahko96 left a comment

Choose a reason for hiding this comment

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

One more small thing

audioapp/views.py Outdated Show resolved Hide resolved
@paulmckissock paulmckissock merged commit e9afe77 into main Jul 26, 2024
1 check passed
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.

2 participants