-
Notifications
You must be signed in to change notification settings - Fork 13
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
hosts drafts #442
hosts drafts #442
Conversation
Signed-off-by: Kial Jinnah <[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.
Temporary Url for review: https://strr-hosts-dev--pr-442-q6k9hp9k.web.app |
:icon="button.icon" | ||
@click="button.action()" | ||
/> | ||
<div> |
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.
wrapped the button in a div to prevent it growing with the width of the row
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.
Temporary Url for review: https://strr-examiner-dev--pr-442-oal1esie.web.app |
@use '@vuepic/vue-datepicker/dist/main.css' as *; | ||
@use 'tailwindcss/base' as *; | ||
@use 'tailwindcss/components' as *; | ||
@use 'tailwindcss/utilities' as *; |
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.
@import depreciated
} else { | ||
selectedCountry.value = { | ||
callingCode: countryCallingCode.value, | ||
label: `+${countryCallingCode.value}` |
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.
when loading from a draft we need this because the api doesn't save the iso2 for the phone number. Was resulting in weird functionality when re-editing an existing person with a phone number
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.
alternatively we could just have these in a util like before. I like this a bit more because I think it's clearer where the 'buttonControl' functionality lives
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.
moved to composable
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.
was able to remove this and build successfully (there was some weird issue before where we needed this component defined in each layer otherwise it would give a build error)
…ix, other fixes Signed-off-by: Kial Jinnah <[email protected]>
Temporary Url for review: https://strr-examiner-dev--pr-442-oal1esie.web.app |
Temporary Url for review: https://strr-platform-dev--pr-442-4zrdrl5t.web.app |
Temporary Url for review: https://strr-strata-hotel-dev--pr-442-727uskx8.web.app |
Temporary Url for review: https://strr-hosts-dev--pr-442-q6k9hp9k.web.app |
Signed-off-by: Kial Jinnah <[email protected]>
Temporary Url for review: https://strr-examiner-dev--pr-442-oal1esie.web.app |
Temporary Url for review: https://strr-strata-hotel-dev--pr-442-727uskx8.web.app |
Temporary Url for review: https://strr-hosts-dev--pr-442-q6k9hp9k.web.app |
Temporary Url for review: https://strr-platform-dev--pr-442-4zrdrl5t.web.app |
Signed-off-by: Kial Jinnah <[email protected]>
Temporary Url for review: https://strr-strata-hotel-dev--pr-442-727uskx8.web.app |
Temporary Url for review: https://strr-examiner-dev--pr-442-oal1esie.web.app |
Temporary Url for review: https://strr-platform-dev--pr-442-4zrdrl5t.web.app |
Temporary Url for review: https://strr-hosts-dev--pr-442-q6k9hp9k.web.app |
async function deleteDraft (row: any) { | ||
try { | ||
deleting.value = true | ||
row.class = 'bg-red-50 animate-pulse' |
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.
slick
? row.address.unitNumber + '-' | ||
: ''}${row.address.streetNumber} ${row.address.streetName}, ${row.address.city}` | ||
: '', | ||
number: row.applicationNumber |
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'm not seeing number as a prop in the locale string?
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.
updated
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 pretty good!
Only concern is on that aria label still. Maybe something like
btn.resumeDraftAria: 'Resume Draft for {number}'. ?
Also need to bump version for all apps.
Great work 👍
Oh oops yeah good catch |
Signed-off-by: Kial Jinnah <[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.
Looks good! 👍
Temporary Url for review: https://strr-strata-hotel-dev--pr-442-727uskx8.web.app |
Temporary Url for review: https://strr-examiner-dev--pr-442-oal1esie.web.app |
Temporary Url for review: https://strr-platform-dev--pr-442-4zrdrl5t.web.app |
Temporary Url for review: https://strr-hosts-dev--pr-442-q6k9hp9k.web.app |
Signed-off-by: Kial Jinnah <[email protected]>
Quality Gate passedIssues Measures |
Temporary Url for review: https://strr-examiner-dev--pr-442-oal1esie.web.app |
Temporary Url for review: https://strr-platform-dev--pr-442-4zrdrl5t.web.app |
Temporary Url for review: https://strr-strata-hotel-dev--pr-442-727uskx8.web.app |
Temporary Url for review: https://strr-hosts-dev--pr-442-q6k9hp9k.web.app |
Issue:
Description of changes:
just waiting on delete draft endpointAll setBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the BC Registry and Digital Services BSD 3-Clause License