Skip to content

Adds number for sms notifications from workflows to /success#5210

Merged
CarinaWolli merged 7 commits intomainfrom
fix/show-sms-reminder-number
Oct 27, 2022
Merged

Adds number for sms notifications from workflows to /success#5210
CarinaWolli merged 7 commits intomainfrom
fix/show-sms-reminder-number

Conversation

@CarinaWolli
Copy link
Copy Markdown
Member

What does this PR do?

  • Show SMS reminder number on /success page with all other booking details
  • Change SMS reminder to SMS notification (it does not need to be a reminder)

Screenshot 2022-10-25 at 17 54 37

Screenshot 2022-10-25 at 17 49 46

Fixes #4813 (#4813 (comment))

Environment: Staging(main branch)

How should this be tested?

  • Create a workflow and add 'send SMS to attendee' as an action
  • Set workflow active on one event type and book this event type
  • See that /success shows the phone number for SMS notifications

@vercel
Copy link
Copy Markdown

vercel Bot commented Oct 25, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
cal ✅ Ready (Inspect) Visit Preview Oct 27, 2022 at 9:53AM (UTC)

Copy link
Copy Markdown
Contributor

@JeroenReumkens JeroenReumkens left a comment

Choose a reason for hiding this comment

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

Works perfect! Only need to do something about the duplicate translation I think. Apart from that ready to approve!

Comment thread apps/web/public/static/locales/en/common.json Outdated
"no_event_types": "No event types setup",
"no_event_types_description": "{{name}} has not setup any event types for you to book."
"no_event_types_description": "{{name}} has not setup any event types for you to book.",
"number_sms_notifications": "Phone number (SMS notifications)"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it hacky to propose adding a non breaking space (\u00a0) between the words SMS and notifications? This will make them "one word" and add the line break after "number". That visually just looks a bit nicer 😁

CleanShot 2022-10-27 at 09 07 04@2x

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wouldn't it be better if we structure it better by moving SMS notifications to a separate tag. That would also allow it to be styled differently(may be a little less bold and lighter color?)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good idea. Let's ask @Jaibles their opinion, since adding additional blocks might also make this look more "chaotic".

Copy link
Copy Markdown
Contributor

@pumfleet pumfleet left a comment

Choose a reason for hiding this comment

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

Looks good to me, pending the comments related to the translation strings

@JeroenReumkens
Copy link
Copy Markdown
Contributor

Looks great to me now @CarinaWolli

@CarinaWolli CarinaWolli enabled auto-merge (squash) October 27, 2022 09:32
@hariombalhara hariombalhara added ♻️ autoupdate tells kodiak to keep this branch up-to-date automerge labels Oct 27, 2022
@CarinaWolli CarinaWolli merged commit b6ae452 into main Oct 27, 2022
@CarinaWolli CarinaWolli deleted the fix/show-sms-reminder-number branch October 27, 2022 09:53
haffla pushed a commit to tourlane/cal.com that referenced this pull request Nov 22, 2022
…5210)

* add sms notification phone number to success page

* remove duplicate translation

* add non breaking space between sms and notifications

Co-authored-by: CarinaWolli <wollencarina@gmail.com>
Co-authored-by: Bailey Pumfleet <pumfleet@hey.com>
haffla pushed a commit to tourlane/cal.com that referenced this pull request Nov 22, 2022
…5210)

* add sms notification phone number to success page

* remove duplicate translation

* add non breaking space between sms and notifications

Co-authored-by: CarinaWolli <wollencarina@gmail.com>
Co-authored-by: Bailey Pumfleet <pumfleet@hey.com>
@PeerRich PeerRich added the core area: core, team members only label Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge ♻️ autoupdate tells kodiak to keep this branch up-to-date core area: core, team members only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CAL-178] Phone number from SMS notifications workflow not available

5 participants