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.

Sign up for GitHub

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

Jump to bottom

Podcast page #26971

Merged
merged 5 commits into from
Aug 15, 2024
Merged

Podcast page #26971

merged 5 commits into from
Aug 15, 2024

Conversation

ndcunningham
Copy link
Contributor

No description provided.

@ndcunningham ndcunningham self-assigned this Jul 17, 2024
@vercel Vercel
Copy link

vercel bot commented Jul 17, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview Aug 15, 2024 5:54pm

@nx-cloud Nx Cloud
Copy link

nx-cloud bot commented Jul 17, 2024 •
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 345f1ed. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 4 targets

Sent with 💌 from NxCloud.

@ndcunningham ndcunningham changed the title Feat/nx dev podcast page Podcast page Jul 18, 2024
bcabanes
bcabanes requested changes Jul 18, 2024
View reviewed changes
nx-dev/data-access-documents/src/lib/blog.api.ts Outdated Show resolved Hide resolved
nx-dev/ui-podcast/src/lib/podcast-list.tsx Outdated
Comment on lines 9 to 11
export function PodcastList({ podcasts }: PodcastListProps): JSX.Element {
return podcasts.length < 1 ? (
<></>
Copy link
Member

Choose a reason for hiding this comment

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

Can you test before calling the component? It will simplify template.

Copy link
Member

Choose a reason for hiding this comment

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

Or do we want to say something when there is no podcast? @juristr

Copy link
Member

Choose a reason for hiding this comment

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

I would put something there, yes. Even though that's not gonna be the case as we're going to push this only once the first podcast edition is created, which Zack is in the process of doing. He has a PR open, I just told him to fix a couple of things first before publishing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added an empty state, aka Stay tuned. @juristr lmk if the wording is fine and we can leave it as is.

juristr
juristr requested changes Jul 18, 2024
View reviewed changes
nx-dev/data-access-documents/src/lib/blog.model.ts Outdated Show resolved Hide resolved
nx-dev/data-access-documents/src/lib/blog.api.ts Outdated Show resolved Hide resolved
nx-dev/ui-podcast/src/lib/podcast-list.tsx Outdated
Comment on lines 9 to 11
export function PodcastList({ podcasts }: PodcastListProps): JSX.Element {
return podcasts.length < 1 ? (
<></>
Copy link
Member

Choose a reason for hiding this comment

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

I would put something there, yes. Even though that's not gonna be the case as we're going to push this only once the first podcast edition is created, which Zack is in the process of doing. He has a PR open, I just told him to fix a couple of things first before publishing

@juristr juristr mentioned this pull request Aug 9, 2024
Closed
@ZackDeRose ZackDeRose mentioned this pull request Aug 15, 2024
Closed
@jaysoo jaysoo merged commit 6d7cdd7 into master Aug 15, 2024
6 checks passed
@jaysoo jaysoo deleted the feat/nx-dev-podcast-page branch August 15, 2024 18:32
jaysoo pushed a commit that referenced this pull request Aug 16, 2024
@ndcunningham @jaysoo
Podcast page (#26971)
9e2bdd3
jaysoo pushed a commit that referenced this pull request Aug 16, 2024
@ndcunningham @jaysoo
Podcast page (#26971)
54b3097
FrozenPandaz pushed a commit that referenced this pull request Aug 19, 2024
@ndcunningham @FrozenPandaz
Podcast page (#26971)
560daa5 
(cherry picked from commit 6d7cdd7)
@github-actions GitHub Actions
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jaysoo jaysoo jaysoo approved these changes

@bcabanes bcabanes bcabanes approved these changes

@FrozenPandaz FrozenPandaz Awaiting requested review from FrozenPandaz FrozenPandaz is a code owner

@vsavkin vsavkin Awaiting requested review from vsavkin vsavkin is a code owner

@juristr juristr Awaiting requested review from juristr

Assignees

@ndcunningham ndcunningham

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ndcunningham @jaysoo @juristr @bcabanes