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

feat(event): implement preview with drafts #1739

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

Conversation

huzaifaedhi22
Copy link
Contributor

@huzaifaedhi22 huzaifaedhi22 commented Feb 2, 2025

📚 Description

This PR

  • adds preview event functionality
  • fixes event photos upload and fetch
  • adds save as draft feature (in browser) for events

📝 Checklist

  • The PR's title follows the Conventional Commit format

@dargmuesli dargmuesli force-pushed the master branch 2 times, most recently from db8592b to 95ec293 Compare February 20, 2025 05:59
Copy link
Member

@dargmuesli dargmuesli left a comment

Choose a reason for hiding this comment

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

Some comments apply to all similar occurrences in other files as well of course

@@ -24,6 +24,7 @@
"test:e2e:docker:server:static": "pnpm --dir tests run test:e2e:docker:server:static"
},
"devDependencies": {
"@internationalized/date": "^3.6.0",
Copy link
Member

Choose a reason for hiding this comment

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

This should go to src/package.json and have no caret

"type": "module"
"type": "module",
"dependencies": {
"@opentelemetry/resources": "^1.30.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 don't think we need to commit this. Issues related to this should likely be fixed by other means.

</template>

<script setup lang="ts">
import { ref, watch } from 'vue'
Copy link
Member

Choose a reason for hiding this comment

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

This line is auto-imported and can be removed

Comment on lines +58 to +63
defineProps<{
modelValue: CalendarDate
placeholder?: string
timePlaceholder?: string
initialTime?: string
}>(),
Copy link
Member

Choose a reason for hiding this comment

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

Please extract this type definition into a Props interface in case we need to import it in another file

Comment on lines +65 to +66
placeholder: 'Select date',
timePlaceholder: 'Select time',
Copy link
Member

Choose a reason for hiding this comment

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

Those strings should be translatable

Comment on lines +399 to +400
preview: Vorschau des Ereignisses
previewHeading: Ereignisvorschau
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
preview: Vorschau des Ereignisses
previewHeading: Ereignisvorschau
preview: Vorschau der Veranstaltung
previewHeading: Veranstaltungsvorschau

@@ -13,6 +13,7 @@
<LayoutBreadcrumbs :items="breadcrumbItems" />
<section>
<LayoutPageTitle :title="t('title')" />

Copy link
Member

Choose a reason for hiding this comment

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

stray empty line

statusCode: 404,
})
// return abortNavigation({ statusCode: 404 })
if (!nuxtApp.isHydrating) {
Copy link
Member

Choose a reason for hiding this comment

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

Isn't isHydrating only true while for a few milliseconds after page load? Maybe you can explain this logic to me in person

Copy link
Member

Choose a reason for hiding this comment

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

Merging in master should reduce the diff here

@@ -138,7 +138,8 @@
"vue-router": "4.5.0",
"vue-tsc": "2.2.0",
"workbox-precaching": "7.3.0",
"zod": "3.24.1"
"zod": "3.24.1",
"@internationalized/date": "^3.6.0"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"@internationalized/date": "^3.6.0"
"@internationalized/date": "3.6.0"

and likely will be moved further up by installing it through pnpm

@dargmuesli dargmuesli changed the title feat(event-preview): implement event preview with drafts feat(event): implement preview with drafts Mar 4, 2025
@dargmuesli dargmuesli changed the base branch from master to main March 7, 2025 10:28
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.

2 participants