actually the utility is to to convert the date to unix timestamp
2023-02-14 04548, 2023
jivte
but the prop is required to send timestamp from one component to another
2023-02-14 04517, 2023
jivte
bcz we have date picker in submitlisten component
2023-02-14 04544, 2023
jivte
and the submit listen function is in addlisten component
2023-02-14 04553, 2023
jivte
thats why the prop is necessary
2023-02-14 04512, 2023
lucifer
akshaaatt: not much progress since the summit afaik.
2023-02-14 04537, 2023
lucifer
mayhem: makes sense to me. is the non-AI approach something we want to do or also an exploratory project for the students??
2023-02-14 04553, 2023
monkey
Right. That touches on my review comment, the timestamp management should be done in the parent component instead and you wouldn't have to pass things back and forth :)
2023-02-14 04502, 2023
jivte has quit
2023-02-14 04523, 2023
jivte joined the channel
2023-02-14 04518, 2023
monkey
At the moment the logic in submit-listen-info is a bit arbitrarily separated from the modal component rather than having a clear separation of which component needs what information. That's why you end up with having to pass data between children and parent, and for the child to react to props changes in componentDidUpdate.
2023-02-14 04533, 2023
monkey
If you are more comfortable with using react hooks, and functional components, we coudl also try to go that direction
2023-02-14 04550, 2023
monkey
(if that's not the case, forget I said anything :p )
2023-02-14 04552, 2023
jivte
monkey: see the details are in one component but while clearing some buttons are in parent component like that cancel button
2023-02-14 04521, 2023
jivte
or the cross button in modal header so passing data forth and back is necessary
2023-02-14 04537, 2023
jivte
if something else could be done could u plz suggest
moreover we have to keep the component more global thats why made those timestamp prop for use in future
2023-02-14 04520, 2023
monkey
If I'm honest, I think you could bring all the code from submit-listen-info into the add-listen-modal, and get rid of all the things that were made irrelevant by that move. That will simplify the components a lot and remove some hacky react code, and we can from there optimize and make reusable components as we need in the future.
2023-02-14 04551, 2023
jivte
I don't think that's a good idea
2023-02-14 04559, 2023
jivte
:|
2023-02-14 04528, 2023
jivte
bcz that would make a very bulky component
2023-02-14 04558, 2023
jivte
could u give me a day so that I could think of something to optimize the code
2023-02-14 04520, 2023
jivte
rest we would go with moving the whole code to addlisten
2023-02-14 04541, 2023
monkey
If you are aiming for reusability, you will need to reslice your cake according to which part does what functionnally. For example a separate isolated timestamp selection component would make sense, but the submit-listen-info currently does too many different things to be reusable well
2023-02-14 04535, 2023
jivte
why don't we make three more components one for search one for details one for timestamp
2023-02-14 04539, 2023
jivte
what u think?
2023-02-14 04550, 2023
jivte
this would ensure reusability
2023-02-14 04508, 2023
jivte
timestamp component could be used in add album feature
2023-02-14 04539, 2023
monkey
I think that will help the code as it stands to separate concerns.
2023-02-14 04559, 2023
jivte
I am still confused
2023-02-14 04511, 2023
jivte
so what should I do finally now?
2023-02-14 04512, 2023
monkey
If you insist on making separate components, then I would have the following components in the modal:
2023-02-14 04512, 2023
monkey
- track search
2023-02-14 04512, 2023
monkey
- track details
2023-02-14 04512, 2023
monkey
- timestamp selection
2023-02-14 04526, 2023
monkey
As you said
2023-02-14 04549, 2023
jivte
I don't insist but I just want the code to be readable for others too bcz all code in one component is a way too bulky
2023-02-14 04520, 2023
jivte
moreover we only have the problem of passing data through props forth and back
2023-02-14 04529, 2023
monkey
I agree, it'll be more readable with separate components.
2023-02-14 04551, 2023
jivte
but with more components comes more props
2023-02-14 04559, 2023
jivte
again same problem
2023-02-14 04501, 2023
jivte
:|
2023-02-14 04522, 2023
monkey
I'm seeing code that shouldn't be there with the way you are using those props, so there are concerns there with the updating and rendering logic
2023-02-14 04527, 2023
monkey
But separate components will help us isolate the issues
2023-02-14 04520, 2023
jivte
I agree but separate components will make logic too much complex
2023-02-14 04521, 2023
monkey
In short, you shouldn't need to use componentDidUpdate. If you're using it that means the structure of the code is wrong
2023-02-14 04514, 2023
jivte
the only solution to not use componentDidUpdate is moving whole code to addlisten
2023-02-14 04557, 2023
monkey
That's why I suggested that :) It is usually a sign that you are doing logic and state management in the wrong place.
2023-02-14 04506, 2023
jivte
bcz in multiple components too bcz of clearing details componetDidUpdate will be used which is not our need
2023-02-14 04520, 2023
jivte
so then shifting the code to addlisten
2023-02-14 04501, 2023
jivte
will ping you after it is done
2023-02-14 04509, 2023
jivte
:)
2023-02-14 04513, 2023
monkey
I think that will be helpful as a base to figure out what can be easily and conveniently separated into a separate component
2023-02-14 04509, 2023
jivte
Yeah separate components could be refactored later on as you said in the review :)
2023-02-14 04507, 2023
monkey
I think we'll find that for example we can use a ListenCard to display the track details, which is already a separate reusable component
2023-02-14 04523, 2023
monkey
But we'll see about that when we refactor
2023-02-14 04559, 2023
jivte shifting the code back
2023-02-14 04548, 2023
jivte
monkey: should I revert the update of the library back
"Kindly be informed that as per the YouTube Developer policies, it is not allowed to play subsequent tracks continuously" surprise
2023-02-14 04522, 2023
reosarevok
Oh well, it was worth a shot
2023-02-14 04517, 2023
monkey
Poop.
2023-02-14 04516, 2023
monkey
I think mayyyybe we can enqueue tracks in the currently active YT player, but that probably means a significant refactor of BrainzPlayer, and losing the ability to use both Spotify and Youtube to play music
2023-02-14 04529, 2023
monkey
What a PITA
2023-02-14 04539, 2023
mayhem
the sooner we give up on YT, the sooner the pain stops.
2023-02-14 04556, 2023
reosarevok
I assume YouTube Music (which is kind of a separate thing) does not help in any way?
2023-02-14 04553, 2023
lucifer
it doesn't have an API afaik
2023-02-14 04549, 2023
mayhem
ok, I think we need to "think different" on the YT front.
2023-02-14 04507, 2023
mayhem
we don't need their permission to play videos. we need their permission to find what we want to play, yes?
2023-02-14 04547, 2023
mayhem
so, we find a way to crawl the data, collect it in MB and then use those links to play content. no API.
2023-02-14 04533, 2023
rdswift
I like it, as long as it doesn't violate any of their Terms of Use.
2023-02-14 04505, 2023
mayhem
probably does. and if it doesn't, then it will violate the terms of service they'll update in 2 weeks time.
2023-02-14 04539, 2023
rdswift
Yeah, I suppose... :-)
2023-02-14 04516, 2023
lucifer
we'll still be using the iframe API to play videos so same terms would still methinks. so yeah still violating.
2023-02-14 04559, 2023
mayhem
but we dont need to be logged in to YT to do that, yes?
2023-02-14 04506, 2023
lucifer
no don't think so
2023-02-14 04507, 2023
mayhem
my take it, we may be violating terms of service, but will they ever know? to me it seems that they are getting their day because we want access to the api.
2023-02-14 04519, 2023
mayhem
I think they have bigger fish to fry than us.
2023-02-14 04513, 2023
rdswift
As long as you're not on their radar now...
2023-02-14 04520, 2023
lucifer
hmm, yeah makes sense
2023-02-14 04529, 2023
mayhem
and what if we are? they block us. then finally not more YT. fine.
2023-02-14 04543, 2023
lucifer
i guess crawling youtube data for addition to MB is a fine thing in itself though.
2023-02-14 04558, 2023
mayhem
I mean where we are is simple: provide a shit experience that doesn't reflect well on us or don't do it all.
2023-02-14 04557, 2023
rdswift
Might be best to decide now to drop them now and save the coding effort?
2023-02-14 04515, 2023
jasje has quit
2023-02-14 04526, 2023
mayhem
that's where I am at now.
2023-02-14 04502, 2023
rdswift
Given all that has transpired, I think that's a prudent decision.
2023-02-14 04522, 2023
mayhem
I'll let monkey make that decision.
2023-02-14 04512, 2023
rdswift
I mean, if they don't want the traffic heading to their site, forget them.
2023-02-14 04531, 2023
mayhem
they don't care. we'd be .00000000000000000000000000000000000000000000000001% of their traffic
i've had various friends at YT try and get a product manager to speak with me. nothing. no one cares. unless you bring a wheelbarrow of cash, they dont give a fuck.
2023-02-14 04548, 2023
mayhem
that sentiment used to be reserved for the music industry, but its the tech industry now too
2023-02-14 04511, 2023
mayhem
I think we may need to pivot on who/what we support.
2023-02-14 04537, 2023
mayhem
keep supporting spotify until they cut that off (only a matter of time).
2023-02-14 04501, 2023
mayhem
so, focus on funkwhale and navidrome. that is where our users are coming from now. lets focus on them.
2023-02-14 04546, 2023
rdswift
Yeah, that article you shared the other day was very insightful on the situation.
2023-02-14 04512, 2023
mayhem
lucifer: on the cover art for playlists -- yes we should support that. and once I finish the cover art maker, then we'll be ready for that!
