"JB is live!" indicator - on page load, call the script to highlight the #livebutton red if jupiter.tube is live.#378
"JB is live!" indicator - on page load, call the script to highlight the #livebutton red if jupiter.tube is live.#378elreydetoda merged 9 commits intoJupiterBroadcasting:mainfrom DECbot:patch-1
Conversation
Adding 'id' attribute to the live button in nav bar for dynamically setting background color when JB is live.
|
Going to close your other PRs (#379 & #380) based on the feedback (in matrix) of consolidating them all into this one:
It won't delete them, but just let others know not to merge them. Also, do you mind putting this PR into draft mode till you do consolidate them all? |
|
Thank you! Also, thank you for wanting to do this feature @DECbot! I can't wait for this feature to be added! This'll be so cool 🥳 🎉 🚀 |
checks the jupiter.tube api for live shows, if count is greater than 0, the #livebutton background-color style is updated to red.
|
I believe this is what you had in mind. Thanks for coaching me through this. |
That's perfect!
Anytime! Just glad to have your contribution 😁 🥳 I still haven't reviewed it yet, but it's looking like it's ready for review now if someone else has some time 🙃 |
|
Screenshot please! |
|
Logic inverted in console for demonstration since jupiter.tube is not live. Formatted in the console as a single liner to appease Firefox.
|
|
That is pretty sexy. Nicely done! |
|
"JoopTubeQuery" ; ) |
gerbrent
left a comment
There was a problem hiding this comment.
Seems ok from my surface-level understanding. Looking for other reviewers to confirm..
|
Would love to get this merged (if acceptable) before Sunday.... |
|
We could also simulate a live show from JB One if needed for testing.... just let me know! |
There was a problem hiding this comment.
Overall I think it's an awesome MVP of what we can do! 🥳
I think I've just got some small feedback on not needed hugo HTML/logic, and also just trying to make the code more DRY.
Once those changes get merged this should be good to go 😁 , thanks again @DECbot and sorry it took so long to review 😅 . Work got pretty crazy this week 🙃 , but thank you for your contribution and I'm super excited about this PR 🥳 🚀 🎉
P.S. I've got extra comment/info + possible future improvements, so do not worry about doing the stuff below (just my inline GH comments)
BTW, here's what it'd look like in the mobile dropdown
Event that I used to test (taken from a while back), and I used Burp Suite Community Edition to intercept and modify the response and test what a "valid" response would be.
{"total":1,"data":[{"id":50,"uuid":"55b2e9d1-ccf3-4914-9f3f-c5d8a59bd4c1","shortUUID":"bzMpxgG2rDKn9C99GSab92","url":"https://jupiter.tube/videos/watch/55b2e9d1-ccf3-4914-9f3f-c5d8a59bd4c1","name":"LINUX Unplugged 471 - We Broke Our Server","category":{"id":15,"label":"Science & Technology"},"licence":{"id":5,"label":"Attribution - Non Commercial - Share Alike"},"language":{"id":"en","label":"English"},"privacy":{"id":1,"label":"Public"},"nsfw":false,"description":"We broke our server, so lets see if we can fix it live before the show is over!","isLocal":true,"duration":0,"views":0,"viewers":19,"likes":0,"dislikes":0,"thumbnailPath":"/static/thumbnails/b610e5f5-bbb5-4ee1-ac24-421c50f0daf5.jpg","previewPath":"/lazy-static/previews/fcd582d8-68c5-4f00-8a4a-0a8ffde1a4a5.jpg","embedPath":"/videos/embed/55b2e9d1-ccf3-4914-9f3f-c5d8a59bd4c1","createdAt":"2022-08-14T16:26:27.938Z","updatedAt":"2022-08-14T18:53:15.544Z","publishedAt":"2022-08-14T18:53:15.543Z","originallyPublishedAt":null,"isLive":true,"account":{"id":3,"displayName":"JBLive Stream","name":"jblive","url":"https://jupiter.tube/accounts/jblive","host":"jupiter.tube","avatars":[{"width":48,"path":"/lazy-static/avatars/e4d17e7f-a144-4a4a-b5d8-b297b06b727f.png","createdAt":"2022-06-07T23:43:56.565Z","updatedAt":"2022-06-07T23:43:56.565Z"},{"width":120,"path":"/lazy-static/avatars/1340f3e9-d0a7-4bc4-bcbd-a8c865eaf1b8.png","createdAt":"2022-05-30T20:36:19.005Z","updatedAt":"2022-05-30T20:36:19.005Z"}],"avatar":{"width":48,"path":"/lazy-static/avatars/e4d17e7f-a144-4a4a-b5d8-b297b06b727f.png","createdAt":"2022-06-07T23:43:56.565Z","updatedAt":"2022-06-07T23:43:56.565Z"}},"channel":{"id":2,"name":"live","displayName":"live","url":"https://jupiter.tube/video-channels/live","host":"jupiter.tube","avatars":[{"width":48,"path":"/lazy-static/avatars/8acfd2a2-ab4e-48aa-990c-3156a2765d2e.png","createdAt":"2022-06-07T23:43:56.607Z","updatedAt":"2022-06-07T23:43:56.607Z"},{"width":120,"path":"/lazy-static/avatars/e9bcfdb7-90a2-479c-b1da-1ab0e5fc9442.png","createdAt":"2022-05-30T20:38:33.632Z","updatedAt":"2022-05-30T20:38:33.632Z"}],"avatar":{"width":48,"path":"/lazy-static/avatars/8acfd2a2-ab4e-48aa-990c-3156a2765d2e.png","createdAt":"2022-06-07T23:43:56.607Z","updatedAt":"2022-06-07T23:43:56.607Z"}}}]}here is the spot that makes it "valid", because the length of the events is greater than 0.
possible future improvements:
- don't make red on live page. (IMO distracts from live show video)
- someway to display (outside of menu (in case people don't click on menu)) to mention that we're live (maybe pop-up once and gently notify them?)
- agree with @ChanceM (source), and this should probably be a class not just an inline style
|
also, @DECbot I saw you were going to push some tests in for this as well (source). I'd honestly make that another PR, and we can work through refining tests for this 😁 . Typically I'd prefer to get test in with the code, but there are some pretty cool feature with playwright were you can intercept and modify the response to "mock" a true answer to the jb endpoint response. Which I think would be really useful in this instance to ensure we can validate the background changing to red. |
Yes, that is my intention, to do a follow-up pull request after this one is accepted to include e2e testing. I didn't know Playwright could intercept API calls--I was intending to inject the JSON response. Intercepting and modifying would be a much better way to do it. I liked the suggestion to do this with classes rather than inline styles. As for a notifier on the mobile layout, I want to figure out how to paint a red dot on the bottom corner of the uncollapsed menu when live much like how Android draws a dot on apps with notifications. For not painting the live button on the live page, I assume Hugo can selectively exclude the doLiveHighlight function call in the live/jb-tube.html partial. That's the laziest solution I can think of at the moment. |
Ya, playwright has a lot of cool features that I'm honestly still learning about 😅 . I've only been using it for about a month or so, but have really been trying to dive in and leverage all it's functionality 😁. For example I just recently learned about that playwright can emulate all kinds of devices based on screen width/height + user-agent 🙂
Ah, now that sounds really cool! 😁 Ya, doing that in a future PR would be pretty awesome 🥳
tl;dr: yes it does 😁 |
elreydetoda
left a comment
There was a problem hiding this comment.
Everything else looks good to me 😁
elreydetoda
left a comment
There was a problem hiding this comment.
Looks good to me 🥳🎉
|
I personally think that it should be more obvious that JB is live, rather than a small red button in the navbar. Something like #34 - should I work on a PR for that? |
|
hmm! I'd be curious to hear what others here think aswell. Has the current LIVE indicator been sufficiently obvious? Happy to reopen this to discussion! |
|
I'm all for the script toggling a css class instead of doing an inline css style change. With a class multiple things can change when live--like not hiding a div with the live episode details. |
|
I admit, I'm bias because I know why that box is red, but I've caught more live shows because of the box than on the old website. |
Adding 'id' attribute to the live button in nav bar for dynamically setting background color when JB is live.