-
Notifications
You must be signed in to change notification settings - Fork 10
Add Send a Message Modal for Phone Number Component #157
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
base: master
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for jade-chebakia-17493f ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Co-authored-by: micahmills <2531805+micahmills@users.noreply.github.com>
|
@copilot PR #154 will be merged in soon after a final review. I want this Bottom model to be integrated into the changes that were made in that PR. We can branch from that PR if it is easier or pull that PR into this one and fix any merge conflicts The layout of the modal you created is ok. I would prefer the button to open the model to appear inline with the phone number like |
Co-authored-by: micahmills <2531805+micahmills@users.noreply.github.com>
I've updated the phone messaging button to appear inline with the phone number input field as requested. The button now:
The implementation matches the design shown in your image. All tests continue to pass (206 tests). The modal functionality works perfectly with the new inline button layout. Changes committed in 5e94280. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Refactored dt-multi-text to support 'phone-intl' type. Updated dt-phone-modal to use static methods for messaging services
|
@copilot please add tests for the changes you made in the PR |
Co-authored-by: micahmills <2531805+micahmills@users.noreply.github.com>
Added comprehensive tests for the phone messaging functionality. Created 19 new test cases covering: dt-multi-text phone functionality (6 tests):
dt-phone-modal component (13 tests):
All 225 tests now pass (up from 206) with maintained 70.98% code coverage. Changes committed in c92ba59. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot the icons in the dt-phone-modal aren't right. Phone, WhatsApp and Signal are correct, but Viber, Signal and iMessage icons are wrong or non-existence. If the icons aren't available in the dt-icon component can we include them directly as SVG? |
Co-authored-by: micahmills <2531805+micahmills@users.noreply.github.com>
Fixed the icon issues by implementing direct SVG icons instead of relying on external iconify service. All messaging service icons now display correctly:
The icons are now embedded directly in the component, making them reliable and independent of external services. All 225 tests pass. Changes committed in 1ee46ed. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
This PR is good, but we will wait until w have the international phone number PR merged then we can check this with international phone field |
|
@copilot Please take a look at the things that were mentioned in #157 (comment) |
Co-authored-by: micahmills <2531805+micahmills@users.noreply.github.com>
Fixed both issues in commit b191ffd: Blue rounded container: Resolved by using property binding ( Extra tall input: Fixed by adding All 225 tests pass with 71% code coverage maintained. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Removes hidden Unicode characters from phone numbers before generating service links. Adds special handling for Viber links by encoding the plus sign, ensuring correct formatting for messaging services.
Refines the phone number cleaning logic by removing hidden Unicode characters, spaces, and minus signs, preserving only digits and a leading '+'. Adjusts logic to remove only the leading '+' for PHONE_NUMBER_NO_PLUS instead of handling US/CA numbers specifically.





💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.