-
Notifications
You must be signed in to change notification settings - Fork 8
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
Components elements subdirectories TypeScript #528
Conversation
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, but I don't see any additional typing? Was that a missed commit, or did I read this PR wrong.
Yes, I add types as soon as the previous PRs get merged (that's why there are still some TODO comments added here). Sorry I should have marked this PR as draft. |
* Bake in the github short sha as the container's release version. (#531) * Bake in the github short sha as the container's release version. * changed github_sha syntax for consistency within the file --------- Co-authored-by: Jo <[email protected]> * Bugs/517 ftue qa (#532) * Don't overwrite app theme, just remove the dark class from html. * * Require one weekday to be selected * Adjust styling of notice bar * Dynamically show warnings on notice bar * Add missing chip * Remove hardcoded cert (#533) * converted ssl cert to a variable * update ssl cert in validate workflow * corrected ssl cert variable reference (#534) * added env variable declaration (#535) * Replace yarn with npm (#537) * Replace yarn with npm * Fix validation workflow * Oops, also swap yarn here. * Add package lock * Waiting List Admin Panel (Readonly) (#536) * Add initial waiting list admin panel. Read-only for now, with some filters. * Add tests * 🌐 Update German translation * Adjust script tag and remove console log --------- Co-authored-by: Andreas Müller <[email protected]> * added recursive flag to frontend zip command (#539) * Add another except for any OAuth2Errors (#540) * Schedule booking confirmation (#519) * ➕ Add schedule booking confirmation * ➕ Enhance schedule API to handle bookings without confirmation * 🔨 Show proper success message depending on confirmation status * 🔨 Make field nullable and fix fn return * 🌐 Update frontend/src/locales/en.json Co-authored-by: Mel <[email protected]> * 🌐 Update German translation * 🔨 Assume appointment mustn't be null --------- Co-authored-by: Mel <[email protected]> * Add types for utils (#520) * Configuration to fix HMR (#527) * ➕ Add configuration to fix HMR * 🔨 Make polling optional by env var * 🔨 Fix merge config * 🔨 Only use necessary options * Update English heading cases (#548) * Use server_default for migrations, and fix a logic error from my suggestion (#549) --------- Co-authored-by: Mel <[email protected]> Co-authored-by: Jo <[email protected]>
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.
@MelissaAutumn I added the types now 😅 I decided to move the props types into a separate interface, let me know your thoughts on this. If you prefer to do type-based declaration of the props, I can change that. From this PR on, I will do it consistently for all further components.
@@ -30,7 +30,7 @@ | |||
:is-selected="d.date === date" | |||
:is-today="d.date === today" | |||
:placeholder="placeholder" | |||
:events="eventsByDate(d.date)" | |||
:has-events="!!eventsByDate(d.date).length" |
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 is just a quick improvement, I don't know how long we will be using our own calendar implementation. The mini month doesn't need the events list anymore, just an event indicator.
@@ -179,7 +179,7 @@ const applyTimezone = (d) => dj(d).utc().tz(dj.tz.guess()); | |||
|
|||
/** | |||
* Generates a list of Qalendar friendly event objects from events and appointments props. | |||
* @type {ComputedRef<*[]>} | |||
* @type {ComputedRef<CalendarEvent[]>} |
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 updated type hints to be used as soon as we're actually typing that component.
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, Thanks!
Description of the Change
This change adds types for all Vue components in
elements/**/*
. For further typing, the previous PRs need to be merged first (hence there are some Todos in this PR).Also the script section was moved to the top in most of the vue files.
Applicable Issues
Implements #413