Skip to content

Conversation

@cnathe
Copy link
Contributor

@cnathe cnathe commented Jan 2, 2026

required: false,
scale: MAX_TEXT_LENGTH,
URL: undefined,
URLTarget: undefined,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the original feature request prefers new URL to default to '_blank'. If not implemented we should probably note that in the issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

after feature verification with Hannah, we decided to not default this checkbox to checked (especially since it starts out disabled until the user enters a URL value)

className="form-control domain-text-option-urltarget"
checked={field.URLTarget === '_blank'}
onChange={this.handleURLTargetChange}
disabled={isFieldFullyLocked(field.lockType) || isEmptyString(field.URL)}
Copy link
Contributor

Choose a reason for hiding this comment

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

I find it strange that if there is an XML setting (which has higher priority), that we still allow editing of target in UI. But maybe that's too much work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I agree but that is the case for all field properties. The field editor doesn't know about any XML metadata overrides (those are applied server side after the fact). The getDomainDetails API gets the domain field info as stored in the DB. It would be odd / confusing if that API returned the XML overrides with the response.

@cnathe cnathe requested a review from XingY January 5, 2026 21:54
@cnathe cnathe merged commit 1237588 into develop Jan 6, 2026
3 checks passed
@cnathe cnathe deleted the fb_urlTargetWindow503 branch January 6, 2026 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants