Skip to content

feat: added download agenda as a pdf #823

Open
harshhgithub wants to merge 34 commits intoasyncapi:masterfrom
harshhgithub:agenda-as-pdf
Open

feat: added download agenda as a pdf #823
harshhgithub wants to merge 34 commits intoasyncapi:masterfrom
harshhgithub:agenda-as-pdf

Conversation

@harshhgithub
Copy link
Contributor

Resolves #553
This PR introduces a fully functional Agenda PDF feature for conference city pages.
Users can now view the agenda directly in the browser and download it as a PDF with consistent styling.
The feature enhances user experience by providing a readable, professional, and portable version of the conference agenda.

Here are some screenshots-

Screenshot (1135)

Example of the Exported PDF's-

Munich-Agenda.pdf
Lagos-Agenda (1).pdf
London-Agenda.4.pdf

@netlify
Copy link

netlify bot commented Oct 22, 2025

Deploy Preview for peaceful-ramanujan-288045 ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit cdcc4e8
🔍 Latest deploy log https://app.netlify.com/projects/peaceful-ramanujan-288045/deploys/698ddbc55c2e6d00088d63de
😎 Deploy Preview https://deploy-preview-823--peaceful-ramanujan-288045.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@harshhgithub
Copy link
Contributor Author

@AceTheCreator @thulieblack can you please review ,

@thulieblack
Copy link
Member

@ashmit-coder @TenzDelek your review as well

Copy link
Member

@TenzDelek TenzDelek left a comment

Choose a reason for hiding this comment

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

Overall looks fine, but I am concern about the code consistency within the repo if we add this. we generally use tailwind through out our codebase but this one doesn't seems to support it. I would research more on the possible alternatives packages or walk through that support those features.
let waits for others opinion as well. I might be wrong here 😄

@harshhgithub
Copy link
Contributor Author

harshhgithub commented Oct 25, 2025

@TenzDelek Thanks a lot for the feedback! 🙏
You’re absolutely right — the rest of the codebase uses Tailwind, and I wanted to stay consistent.
However, since @react-pdf/renderer doesn’t render HTML (it creates a virtual PDF tree instead of DOM elements), it doesn’t support Tailwind classes or utility-based styling.

I looked into alternatives like react-pdf-tailwind and @react-pdf/styled-components, but they’re still limited or experimental for production-level use.

That’s why I went with StyleSheet.create() here — it’s the officially supported approach for PDFs and scoped only within this component, so it won’t affect any Tailwind-styled parts of the app.

@harshhgithub
Copy link
Contributor Author

@TenzDelek @AceTheCreator pls review

Copy link
Member

@AceTheCreator AceTheCreator left a comment

Choose a reason for hiding this comment

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

Left you some reviews :)

</View>
<View style={styles.speakerCol}>
{Array.isArray(item.speaker)
? item.speaker.map((id: any, j: React.Key | null | undefined) => {
Copy link
Member

Choose a reason for hiding this comment

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

The speaker ID can't be a type of any. It has to be a type of number

</View>
<View style={styles.speakerCol}>
{Array.isArray(item.speaker)
? item.speaker.map((id: any, j: React.Key | null | undefined) => {
Copy link
Member

Choose a reason for hiding this comment

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

j: React.Key | null | undefined

This is a redundant line since the speaker id can also be used as a key, they're unique numbers :)

})
: (() => {
const sp = city.speakers.find(
(s: any) => s.id === item.speaker
Copy link
Member

Choose a reason for hiding this comment

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

Please, avoid using any when a type exists. The speaker object has an interface; could you use the appropriate type construct?

);
};

export const PdfViewer = ({ city }: { city: any }) => (
Copy link
Member

Choose a reason for hiding this comment

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

{ city: any }

City has an interface too :)

</div>
);

export const PdfDownloadButton = ({ city }: { city: any }) => (
Copy link
Member

Choose a reason for hiding this comment

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

{ city: any }

Use the appropriate city type instead of 'any'.

className="w-full md:w-[200px] px-10 py-3"
disabled={loading}
>
{loading ? 'Preparing PDF…' : 'Download PDF'}
Copy link
Member

Choose a reason for hiding this comment

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

I think we shouldn't use the word 'Download PDF' instead use 'Download Agenda'

@harshhgithub
Copy link
Contributor Author

@AceTheCreator i have made the requested changes , thankyou for your valuable feedback and reviews

Copy link
Member

@AceTheCreator AceTheCreator left a comment

Choose a reason for hiding this comment

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

Left you a final review!

className="w-full md:w-[200px] px-10 py-3"
disabled={loading}
>
{loading ? 'Preparing Agenda…' : 'Download Agenda'}
Copy link
Member

Choose a reason for hiding this comment

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

The loading text is breaking onto a new line, and I think it’s caused by the button’s [200px] width.

@harshhgithub
Copy link
Contributor Author

@AceTheCreator fixed it .

Copy link
Member

@AceTheCreator AceTheCreator left a comment

Choose a reason for hiding this comment

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

@harshhgithub one final thing i'm thinking of is, do we really want the download agenda button visible for past events?

cc @thulieblack

@harshhgithub
Copy link
Contributor Author

@harshhgithub one final thing i'm thinking of is, do we really want the download agenda button visible for past events?

cc @thulieblack

We could hide it for events marked as “completed” to keep the UI cleaner.
What do you think, @thulieblack?

@thulieblack
Copy link
Member

Yes, if you can hide it once event is finished that would be great

@harshhgithub
Copy link
Contributor Author

@thulieblack @AceTheCreator i have made the “Download Agenda” button hidden for past events and verified that the button appears for future event dates .

Copy link
Member

@AceTheCreator AceTheCreator left a comment

Choose a reason for hiding this comment

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

Left you some review :)

Comment on lines 12 to 21
const isPastEvent = (dateString: string): boolean => {
if (dateString.includes('-')) {
const parts = dateString.split('-').map(p => p.trim());
const endDateString = parts[parts.length - 1];
const endDate = new Date(endDateString);
return endDate < new Date();
}
return new Date(dateString) < new Date();
};

Copy link
Member

Choose a reason for hiding this comment

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

I think we should add a helper function to check whether an event is in the past, so it can be reused throughout the application. Right now, a new check is created each time it’s needed (for example, the tickets section also checks for past events).

package.json Outdated
"@googleapis/sheets": "^7.0.0",
"axios": "^1.12.0",
"@react-pdf/renderer": "^4.3.1",
"axios": "^1.8.2",
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, where's axios being used in your implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes axios isn’t used in this implementation.
It was unintentionally left in package.json and can be removed.

@harshhgithub
Copy link
Contributor Author

@AceTheCreator @thulieblack please have a look

@harshhgithub
Copy link
Contributor Author

@AceTheCreator if i remove the axios it causes checks failing , axios was updated when i run npm run build

Copy link
Member

@AceTheCreator AceTheCreator left a comment

Choose a reason for hiding this comment

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

Left you an additional suggestion

Comment on lines 1 to 21
export const isPastEvent = (dateString: string): boolean => {
const today = new Date();
today.setHours(0, 0, 0, 0);

if (dateString.includes('-')) {
const parts = dateString.split('-').map((p) => p.trim());
const endDateString = parts[parts.length - 1];
const endDate = new Date(endDateString);
if (isNaN(endDate.getTime())) {
return false;
}
endDate.setHours(0, 0, 0, 0);
return endDate < today;
}
const parsedDate = new Date(dateString);
if (isNaN(parsedDate.getTime())) {
return false;
}
parsedDate.setHours(0, 0, 0, 0);
return parsedDate < today;
};
Copy link
Member

Choose a reason for hiding this comment

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

@harshhgithub I noticed we already have a utility function called status that checks whether an event is ONGOING, UPCOMING, or ENDED. I think it would be better to reuse that instead of creating a new helper function. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@harshhgithub I noticed we already have a utility function called status that checks whether an event is ONGOING, UPCOMING, or ENDED. I think it would be better to reuse that instead of creating a new helper function. What do you think?

One thing I noticed though is that the current implementation doesn’t fully handle date ranges like 2 - 3 July, 2025. Splitting on - results in an invalid start date (new Date("2")), which can cause runtime issues during build/deploy.

I’m happy to refactor getEventStatus to safely parse ranged dates (derive proper start/end dates) and then reuse it instead of introducing isPastEvent. Let me know if that approach works for you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, i'd appreciate it if you extend the getEventStatus method. That'd be all for me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AceTheCreator @thulieblack I extended getEventStatus to handle date ranges correctly, reusing isPastEvent to keep the logic consistent across the codebase.

Copy link
Member

@AceTheCreator AceTheCreator left a comment

Choose a reason for hiding this comment

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

I think it will also good for you to include a unit test that can be used to validate these utility functions 🙏🏽

Comment on lines 14 to 16
if (isNaN(startDate.getTime())) {
return ConferenceStatus.ENDED;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I notice a bug here... For range dates like "9 - 11 December, 2026", parts[0] is just "9". V8 silently parses new Date("9") as Sept 1, 2001 instead of returning Invalid Date, so the isNaN guard is skipped and future events incorrectly return ONGOING.

Fix: when parts[0] is just a number, borrow the month/year from the end part to form a valid start date (e.g. "9""9 December, 2026").

Copy link
Contributor Author

@harshhgithub harshhgithub Jan 28, 2026

Choose a reason for hiding this comment

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

@AceTheCreator I’ve updated getEventStatus to normalize numeric start days by borrowing the month/year from the end date (e.g. "9 - 11 December, 2026"), which avoids the V8 parsing issue.

I’ve also added a small unit test alongside utils/status.test.ts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AceTheCreator pls review

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.

[FEATURE] Agenda Enhancements ( download agenda as PDF )

4 participants