Allow test support phone in dev mode #7851
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: +145 B (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
|
Compatibility tests failing due to known issue: p1701263468047559-slack-CGGCLBN58 |
htdat
reviewed
Dec 14, 2023
|
|
||
| const [ isPhoneValid, setPhoneValidity ] = useState( true ); | ||
| if ( ! isPhoneValid && ! isEmptyPhoneValid ) { | ||
| if ( ! isPhoneValid && ! isEmptyPhoneValid && ! isDevModeEnabled ) { |
There was a problem hiding this comment.
This skips the phone check completely in the dev mode, and it's not ideal per these examples:
- +1 000 does not work.
- +1 0000 works.
- +1 1234 does not work.
Could we be more specific in checking this in the dev mode? I think it's also a nice idea to suggest using 0000 directly in the supportPhoneError such as:
Please enter a valid phone number. Since you're using Dev Move, you can also use this test number
0000.
There was a problem hiding this comment.
To make it more simple, may just use support phone label to carry out this message, and keep your current code.
Something like this https://gist.github.com/htdat/996a4178d630335d85636f3f979958e2
Update: My point here is that people in dev mode do not know which number to use as we no longer have validation because they might not always start with 0000.
|
Modified the PR to accept the test number in dev mode. @htdat |
|
Add hint text in label 
|
htdat
approved these changes
Dec 15, 2023
| Significance: patch | ||
| Type: fix | ||
|
|
||
| Allow test support phone in dev mode |
There was a problem hiding this comment.
May update the title of the PR too.
There was a problem hiding this comment.
Not sure what to change here? Allow "test phone number" (as Stripe calls it)
for support phone number in "dev mode"
I've tried to modify it to make it more clear.
There was a problem hiding this comment.
🤦 @jessy-p - sorry for the confusion, I meant the title of this PR, not the changelog. I left the comment here so that you can pick up the same changelog entry. I would do better next time.
Currently, it's Skip support phone validation in dev mode. You would update it to Allow test support phone in dev mode.
jessy-p
changed the title
|
It seems this PR was merged despite having errors in the JS tests, which are now present in the |
6 tasks
6 tasks
|
👋 I am looking at this issue, thanks @rafaelzaleski |
6 tasks
|
PR to fix the JS test: #7930 It seems the issue was still since the start of this PR but it was buried among other Woo beta compatibility tests. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #6782
Changes proposed in this Pull Request
Testing instructions
npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge