Conversation
|
Hey @devmount For me when I run the branch locally, after I confirm a time slot selection, the booking confirmed dialog doesn't appear - I just get a blank screen (see screenshot). Before running I did a
|
Ignore that - looks like you did another commit right after I pulled your branch; I pulled the latest and the dialog appears now \o/ looks great!
|
|
Just FYI this breaks the book appointment E2E tests as can be expected; I have created #1431 to update the test accordingly and will take that on. |
There was a problem hiding this comment.
Looking good so far! Just a few comments on some bits:
Other than the inline comments in the code, there a few mismatching typography that I captured:
- This text seems to be larger and with a different color than the Zeplin
- Let's double check the colour of the description here, it seems to be
--colour-ti-basein Zeplin but the current implement looks slightly different
- As noted in your PR description, the "Subscribe" button is a tad different from what we currently have in
services-uibut that can be done as a one-off change by overriding the styles, for example:
:deep(.base.filled.primary) {
// your overridden styles here
}
| padding: 1.5rem 1.5rem 3.5rem; | ||
| max-width: 22rem; | ||
|
|
||
| background-image: radial-gradient(circle at 100% 100%, #336d71, #1b222e); |
There was a problem hiding this comment.
This looks a bit off from the mockup. As a heads up, Zeplin unfortunately is not very accurate on the gradients overall :( but I think we can more closely approximate this by tweaking the pivot point of the gradient somewhere to the bottom right instead?
This one is not a blocker though as it might be time consuming / not super worthy to spend time now on it IMHO.
In this PR:
frontend/src/views/BookerView/components/BookingViewSuccess.vue
Outdated
Show resolved
Hide resolved
frontend/src/views/BookerView/components/BookingViewSuccess.vue
Outdated
Show resolved
Hide resolved
frontend/src/views/BookerView/components/BookingViewSuccess.vue
Outdated
Show resolved
Hide resolved
|
@devmount Ah, unsure if you tackling #1423 (comment) as well on this PR but it seems that it is currently behaving the same as the original issue description:
The header / footer on mobile are preventing the Booking Confirmation UI to be displayed fully |
|
@rwood-moz @davinotdavid Thanks so much for the ultra fast reviews ⚡ ❤️ I'll fix the things you mentioned tomorrow (for me) 🙏🏻 |
Sorry about that @rwood-moz. I forgot to commit a file and quickly pushed it, but only after I assigned you as reviewers 😇 You're simply too fast 😂 🙇🏻 |
Thanks so much Rob! For always keeping the tests in sync 🙏🏻 |
|
@davinotdavid Wow, thanks so much for your thorough review!! Much appreciated!
|
Yes, I decided to do that in a separate issue, since it seems to affect a lot of pages / is kind of a general problem, how the footer is currently implemented. I created #1432 for this, let's discuss the general page layout there. @davinotdavid Actually nevermind. I just went through the application again and it really seems to be the only place where this is happening. I'll fixed it here, sorry for the confusion 😅 |
davinotdavid
left a comment
There was a problem hiding this comment.
Thanks for the revisions! It looks great!
The only bit left is that the Download ICS button doesn't seem to be working well. We can refer to the implementation in the Bookings page / SlidingPanel as the Download ICS there seems to be working well!
|
@davinotdavid I fixed the ICS download now. The problem was, that we need |
|
Downloading the ICS (from the booking confirmation dialog) worked for me with your latest updates \o/ |
davinotdavid
left a comment
There was a problem hiding this comment.
Awesome! Works well, thank you so much for the great work an patience on the review points! 🎉





Description of the Change
Screenshots
Light mode, logged in:

Dark mode, anonymous:

Benefits
Known issues / Things to improve
Applicable Issues
Closes #1403
Closes #1423
Closes #1432