Fix invalid hostnames on platform creation #1652
Conversation
|
The preview deployment is in progress. 🟡 Last updated at: 2025-02-27 14:29:14 CET |
|
The preview deployment is in progress. 🟡 Last updated at: 2025-02-27 14:29:15 CET |
|
The preview deployment is in progress. 🟡 Last updated at: 2025-02-27 14:29:16 CET |
|
The preview deployment is in progress. 🟡 Last updated at: 2025-02-27 14:29:17 CET |
| // double-check the hostname value. | ||
| if (platform === PlatformType.Flutterweb && !isHostnameValid($createPlatform.hostname)) { | ||
| throw new Error('Please enter a valid hostname'); | ||
| } | ||
|
|
There was a problem hiding this comment.
I thought we typically pass a regex to the input for these kinds of validations. Why can't we do that here?
There was a problem hiding this comment.
- extracted function can be helpful for future
- We could use regex pattern, but this way also helps in disabling the next button
There was a problem hiding this comment.
Re-usable is good.
I still have some concerns:
- This pattern is different compared to other places in the Console.
- There is little feedback to the user about what's wrong.
For example, here:
Yes, the button is disabled, but I don't know why. Compared to other places like create document:
As you can see for the asdf attribute, there's more feedback to the developer about what's wrong.
There was a problem hiding this comment.
Makes sense. I can add a regex with the existing logic and that should give a clearer idea on why its disabled. Thanks for the screenshots 👍
There was a problem hiding this comment.
@stnguyen90 made a few changes to overall logic. It doesn't seem possible to directly manage the warning helper via the InputText due to regex complexity as Svelte breaks it. Let me know if the newer changes are OK (backwards compatible).
There was a problem hiding this comment.
Ya, we've dealt the regex problem in the past. It seems there's a special type of regex that's used in HTML inputs. See 4c416c7
There was a problem hiding this comment.
What if we update the inputText input to expose a pattern and patternError like inputOTP:
console/src/lib/elements/forms/inputOTP.svelte
Lines 12 to 13 in e5ab426
As for the pattern, I think this should work:
const pattern = String.raw`(\*\.)?(?!-)[A-Za-z0-9\-]+([\-\.]{1}[a-z0-9]+)*\.[A-Za-z]{2,18}`;
We might also want to create a new input because we're going to also need it on the update platform pages such as:
There was a problem hiding this comment.
That regex wouldn't have allowed *, updated with latest changes. No changes to wizard required now.
| // double-check the hostname value. | ||
| if (platform === PlatformType.Flutterweb && !isHostnameValid($createPlatform.hostname)) { | ||
| throw new Error('Please enter a valid hostname'); | ||
| } | ||
|
|
There was a problem hiding this comment.
What if we update the inputText input to expose a pattern and patternError like inputOTP:
console/src/lib/elements/forms/inputOTP.svelte
Lines 12 to 13 in e5ab426
As for the pattern, I think this should work:
const pattern = String.raw`(\*\.)?(?!-)[A-Za-z0-9\-]+([\-\.]{1}[a-z0-9]+)*\.[A-Za-z]{2,18}`;
We might also want to create a new input because we're going to also need it on the update platform pages such as:
What does this PR do?
Fixes an issue where invalid hostnames were allowed for web platforms.
Test Plan
Manual.
Related PRs and Issues
Have you read the Contributing Guidelines on issues?
Yes.