Custom Domain: Allow updating domains for existing sites. #1064
Conversation
youknowriad
force-pushed
the
add/allow-updating-custom-domain
branch
from
2c7503d to
c1eb800
Compare
|
Can I get some reviews here? |
youknowriad
changed the title
bcotrim
left a comment
bcotrim
left a comment
There was a problem hiding this comment.
Great feature!
Added some comments, but overall I think this looks great!
Something I noticed during my tests:
- Having an invalid domain disables the submit, even when "Use custom domain" isn't checked. I recorded on "Edit Site" but the same behavior happens on "Add Site":
Screen.Recording.2025-03-19.at.18.28.37.mov
src/lib/hosts-file.ts
Outdated
| } | ||
| }; | ||
|
|
||
| export const replaceDomainInHosts = async ( |
There was a problem hiding this comment.
I would rename this function as it handles all possible domain change scenarios.
And to keep the format consistent, move the replacing logic to its own replaceDomainInHosts
There was a problem hiding this comment.
Not sure I understand this comment fully but I renamed the function.
| ( selectedSite?.phpVersion as AllowedPHPVersion ) ?? DEFAULT_PHP_VERSION | ||
| ); | ||
| const [ selectedWpVersion, setSelectedWpVersion ] = useState( currentWpVersion ); | ||
| const [ useCustomDomain, setUseCustomDomain ] = useState( Boolean( selectedSite?.customDomain ) ); |
There was a problem hiding this comment.
It seems a lot of this logic is very similar to the one we added to site-form
I believe there's an opportunity here to create a shared component/hook, to avoid repetition.
There was a problem hiding this comment.
Entirely agree but this is not new. The way the form works today is not great and would deserve a refactor but it's not a small change.
|
I fixed the validation bug. |
wojtekn
changed the title
bcotrim
left a comment
bcotrim
left a comment
There was a problem hiding this comment.
Thanks for making the changes.
I added a small comment, but it shouldn't be blocking.
I noticed the custom domain error message get cut when we display multiple errors.
This should be fixed with this change but something to re-test later.
@bcotrim I tested it when I rebased #1078 and noticed it is fixed in Add Site, but it still happens in Edit Site. Are you also reproduce it there? |
I don't see it happening on "Add Site" or "Edit Site". Screen.Recording.2025-03-21.at.10.01.25.mov |
Oh, it was that. Thanks for clarifying. When I edit the site name, then enable a custom domain and enter an invalid one, Save button toggles correctly when I keep disabling and enabling a custom domain. |
Related issues
Summary
Testing instructions