-
Notifications
You must be signed in to change notification settings - Fork 6
angel/archive page #39
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
angel/archive page #39
Conversation
|
IRL reviews done by Ezekiel and AJ at the DevX meet on 9/21/25 |
|
Looks good to me. Really simple, a good start to showcasing past speakers. |
@samholmes Here's a screen capture above. You have any notes before merging? |
|
My concern is that because the stream data is static, it adds more overhead to the crew to update the stream list. Perhaps it would be viable to connect to some YouTube public API to get the videos from a playlist sorted by the recency. This way YouTube becomes the data storage layer and devx.network is just a view layer to that. A comment on the presentation, I'd like to see the background be black or transparent instead of introducing a new color. |
|
@samholmes valid concern. How about I manually enter data for now, and we can automate via YouTube api in a follow up pr? @AngelCodes95 You good to make the presentation adjustment Sam mentioned? |
|
@tomatrow I'm okay with this proposal so long as you own the manual work. |
|
@samholmes I shall own this so hard |
Absolutely! I'll get cracking on that tonight when I get home. Totally agree on the API point as well. We talked about both routes at the coffee coworking event and AJ sold me on the progressive enhancement approach with static data to start. Thanks for taking time to review! |
a1a7e2b to
c8dc7ba
Compare
|
Just buttoned that up, AJ pointed out a second element that go that new gray color and its also fixed. Archive card background transparent, date label is black. In regard to the API idea Sam brought up, who would have access to the cloud console to get keys made etc? I'd be happy to make another branch and explore adding the API functionality after this. |
|
Thank you both for the feedback and guidance! Highly appreciated! |
|
This looks a lot better. I have some more general product feature feedback here. I'd like to see this page be called "Watch" instead of "Archive". A nice organization of the page should be sectioned by year (2024, 2025, etc). In each section a grid of YouTube video thumbnails. Clicking on the thumbnail takes you to the YouTube video on youtube.com rather than having an embed. The reason for this is to drive traffic to our channel for liking, commenting, subscribing, etc. We already have playlists that could be better organized: https://www.youtube.com/@DEVxNetwork/playlists And I was working on creating the dedicated talk videos for each speakers presentation. I believe the content should be just of the speakers and not the whole live stream because the introduction, although apart of the live stream, isn't really important content for the web. Here's an example of what that grid might look like. This would be my ask from a product perspective, before reviewing the actual implementation. |
|
That is very helpful and a good idea! I'll get to work on implementing that and see what you think. |
Tried to upload a screen recording demo but couldn't get it under 10mb's. Hopefully this is closer to what you envisioned, @samholmes |
|
This commit was more for the layout and design, I will go through manually and add start and stop times for all the thumbnails when I have a spare moment to watch each one. For now they just direct you to the livestream. |
|
Curious what you guys had in mind for thumbnails. I made it uniform for the screenshots. On the Channel the current videos in 2025 all have the uniform thumbnail and 2024 have individual thumbnails. I pushed a commit that starts each video at the first speakers talk. In the static data I included end times for the talks to make playlist creation easier in the future. Since we moved away from embeds I can't control when the user stops watching the video until we take advantage of the playlist idea sam mentioned with each speakers individual talks. |
samholmes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heres' some feedback from what I reviewed so far.
app/watch/page.tsx
Outdated
| return url | ||
| } | ||
|
|
||
| const generateHeroDescription = (talks: Talk[]) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function can be inline since it's only used in one place.
app/watch/page.tsx
Outdated
|
|
||
| // Functions | ||
|
|
||
| const buildYouTubeUrl = (id: string, startTime?: string) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Utility functions like this are better suited at the bottom of the exported component and below the styled components. The first thing in a file should be interfaces (what is this thing) and then implementation (how is this thing) where exports proceed non-exports.
app/watch/page.tsx
Outdated
| .sort((a, b) => b - a) | ||
|
|
||
| // Get the most recent video across all years | ||
| const allVideos = [...livestreams].sort( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think sort is expensive enough it warrants memoization for allVideos and years
| <ErrorBoundary | ||
| fallback={<div style={{ backgroundColor: "black", width: "100%", height: "100%" }} />} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of the scope of this PR, but we should really have this ErrorBoundary in the PotionBackground component.
| <HeroSection> | ||
| <HeroContent> | ||
| <HeroHook>Check out our most recent talk:</HeroHook> | ||
| </HeroContent> | ||
| <HeroVideoCard> | ||
| <HeroThumbnailSection> | ||
| <HeroThumbnailContainer> | ||
| <HeroVideoCategory>{mostRecentVideo.title}</HeroVideoCategory> | ||
| <ThumbnailLink | ||
| href={buildYouTubeUrl(mostRecentVideo.id, mostRecentVideo.startTime)} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| > | ||
| <HeroThumbnailImage src="/images/DEVxCover.png" alt="" /> | ||
| <HeroPlayButton>▶</HeroPlayButton> | ||
| </ThumbnailLink> | ||
| </HeroThumbnailContainer> | ||
| </HeroThumbnailSection> | ||
| <HeroVideoContent> | ||
| <HeroVideoDescription> | ||
| {generateHeroDescription(mostRecentVideo.talks)} | ||
| </HeroVideoDescription> | ||
| </HeroVideoContent> | ||
| </HeroVideoCard> | ||
| </HeroSection> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to see the design where the HeroHook is a HeroHeading at the top and show the 1-3 most recent video(s). Soon we'll have our talk videos published individually and instead of only as Live Streams (we have some individual talks video already!), so having the last event's talks shown here would be the way to go.
app/watch/page.tsx
Outdated
| rel="noopener noreferrer" | ||
| > | ||
| <ThumbnailContainer> | ||
| <ThumbnailImage src="/images/DEVxCover.png" alt="" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the cover image, we should be using the youtube video's thumbnail: https://chatgpt.com/share/68e180cb-5600-800e-a6ed-7dece21e3af8
| <YearHeader>{year}</YearHeader> | ||
|
|
||
| {/* Render all remaining videos in grid */} | ||
| <LivestreamGrid> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This stream grid should support individual video's with one speaker per video. Perhaps limit the video list to the ones that are individual talk videos for now and omit the live stream videos. I really want to show case our videos as individual talks, not live streams.
|
Thank you for taking the time to review and provide feedback @samholmes! I'll tackle each over the next couple days as I have free moments! Do you have commit preferences? For example, each review comment addressed in its own commit. |
|
@AngelCodes95 I prefer atomic commits with conventional commit standard formatting. The words "atomic commit" can be misunderstood to mean "smallest change possible" in the context of code change, but really it's in the context of "idea change". The best way to think about it is in "paragraphs" not "sentences". So a commit is a paragraph getting at one point and that point is the one semantic change. Though I'm loose on the commit format for repo because it's more about results than practice. If you want to exercise your rebasing skills, by all means, but if you add |
Add archive page with embedded YouTube videos, livestream data structure, and header navigation. Styled to match site theme.
Pivot from livestream-centric to speaker-centric model. Create talks.ts with flat structure where each talk is a separate entry with timestamps. Update UI to display one card per speaker. Rename archive to watch and add all 31 talk timestamps.
6b6c784 to
2a96958
Compare
|
This feels more aligned with your atomic commit wishes, I could also have the refactor be its own commit to show that organizational change separately from the feature work. |
samholmes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! One final change required about the commit message for the last commit not matching.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change this commit message to match the commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed! Good catch. I'll bundle config changes with their related feature work next time.
2a96958 to
2f2f8e0
Compare
|
Woohoo 🥳 Thanks for the guidance and feedback, gentlemen! |
|
@AngelCodes95 Thanks for the PR! |
|
What up guys! I seen one of you on FB and am just chiming in. I'm new to everything and am still learning. What are you guys building? I'm trying to get actions to run workflow to update websites to their assigned geographical location. Having a fudge of a time but earning every bit of it. Four days later and about 75 workflows I am learning much .....lol |
|
@TitanBusinessPros I didn't know we were on Facebook. Do you, perhaps, identify as an llm? |
|
Someone was on a Claude.ai post talking about something. I don't understand much of the terminology and am new at software development. Just trying to make friends. |
no worries! check out the devx page by googling "devx san diego" it will be first link... this pull request is related to the watch tab you'll see at the top or within the nav on mobile. Just added a way for folks to watch various individual talks given at our monthly meetup. If you are early in your journey and want to learn without too much investment up front I found a lot of value through freecodecamp and the khan academy. Hope that helps you on your journey. Best way to make friends will be in person meetups, that's how I met these folks and added this PR. I found the meetup by searching local tech meetups. |
|
@TitanBusinessPros We have facebook page, but we haven't done much with it. I'm happy to hear you found us. You can find everything you need to know about DEVx at https://devx.network If you're local to San Diego, we'd love to have you attend one of our events! |







Summary
Changes
/archivepage with embedded YouTube videosapp/info/livestreams.ts