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.

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

Add the new bookings system #653

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add the new bookings system #653

wants to merge 3 commits into from

Conversation

ilgazer
Copy link
Contributor

@ilgazer ilgazer commented Jan 12, 2025

I added the new bookings system as an iframe, and reverted all of the booking-related links to their old state.

Copy link

cloudflare-workers-and-pages bot commented Jan 12, 2025

Deploying outsite-nl with  Cloudflare Pages  Cloudflare Pages

Latest commit: afdd09d
Status: ✅  Deploy successful!
Preview URL: https://286bd799.dwhdelft-nl.pages.dev
Branch Preview URL: https://add-new-bookings.dwhdelft-nl.pages.dev

View logs

Copy link

Deploying dwhdelft-nl with  Cloudflare Pages  Cloudflare Pages

Latest commit: afdd09d
Status: ✅  Deploy successful!
Preview URL: https://9ec0c5d1.dwhdelft-nl-u1a.pages.dev
Branch Preview URL: https://add-new-bookings.dwhdelft-nl-u1a.pages.dev

View logs

@ilgazer ilgazer requested review from thomcsmits and vanadie January 12, 2025 16:27
@casperboone casperboone self-requested a review January 12, 2025 16:44
Copy link
Member

@thomcsmits thomcsmits left a comment

Choose a reason for hiding this comment

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

Very nice new system!

Two things I quickly found:

  • Visually, this white triangle isn't fantastic. Maybe we can make this also the same purple?
    image
  • Am I correct that the booking system is only in English? On the Dutch page, the iFrame is still in English

@ilgazer
Copy link
Contributor Author

ilgazer commented Feb 5, 2025

What do you mean by the white triangle? Also, I forgot to add dutch support but can do it without much difficulty. Also, do we want a way to put separate english and dutch title and descriptions?

@thomcsmits
Copy link
Member

@ilgazer

I mean this triangle, maybe we can also make it purple?
Screenshot 2025-02-13 at 11 22 51 AM

Yeah, I think separate english/dutch is good in the booking app? Whatever you think is easiest!

@@ -73,7 +72,7 @@
/song https://www.youtube.com/watch?v=DjZtKkhZtOE
/sponsorkliks https://www.sponsorkliks.com/partners.php?club=4338
/sportsparty https://www.facebook.com/events/287302566914776
/studieplekken /
/studieplekken /book
Copy link
Member

Choose a reason for hiding this comment

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

should not be restored

@@ -84,7 +83,7 @@
/vertrouwenspersonen https://dwhdelft.nl/lets-talk
/volunqueers https://docs.google.com/spreadsheets/d/1FTK74hMFVRL2j1opFAi-RCeZTooJrNfnG3dzCQFddOU/edit#gid=0
/vrijwilligersprijs /vrijwilligersprijzen
/weekvandeontmoetingdiner /
/weekvandeontmoetingdiner /book
Copy link
Member

Choose a reason for hiding this comment

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

should not be restored

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 it's better to make this an EatingOUT-specific page, with information, pictures, and details, and step away from the idea of a general booking page.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe make these external links, like the external link to outsite.nl from dwhdelft.nl in the nav bar

Copy link
Member

Choose a reason for hiding this comment

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

i'd prefer /eatingout, so it change in the future

const route = useRoute()

onMounted(() => {
window.addEventListener('message', (event) => {
Copy link
Member

Choose a reason for hiding this comment

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

smart!

onMounted(() => {
window.addEventListener('message', (event) => {
if (event.data.source === 'size_alert') {
const asd = document.querySelector('iframe')
Copy link
Member

Choose a reason for hiding this comment

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

let's pick a declarative name here

Comment on lines +50 to +55
<div v-if="route.query.canceled" class="mb-6 flex items-center space-x-4 rounded bg-brand-100 p-4 text-lg">
<ElementsIconCircle class="p-5">
<IconInformationOutline class="size-6" />
</ElementsIconCircle>
<h4 class="text-xl leading-tight" v-text="t('canceled')" />
</div>
Copy link
Member

Choose a reason for hiding this comment

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

Are you still using this in your project?

if (event.data.source === 'size_alert') {
const asd = document.querySelector('iframe')
asd.height = event.data.height
asd.width = event.data.width
Copy link
Member

Choose a reason for hiding this comment

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

Does dynamic width render well?

</script>

<template>
<LayoutSmallHeader bg="bg-brand-450">{{ t('title') }}</LayoutSmallHeader>
Copy link
Member

Choose a reason for hiding this comment

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

This is triangleClass="border-brand-450" now instead of bg=

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.

3 participants