Skip to content

fix(web-domains): fix province select #22561

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.

Sign up for GitHub

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

Open
louisbensiovh wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(web-domains): fix province select #22561

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 77 additions & 25 deletions packages/manager/apps/web-domains/src/domain/components/ContactEdit/EditHolderFormField.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,24 @@ const COUNTRY_LABELS = new Set([
FIELD_NAME_LIST.nationality,
]);

const AREA_LABELS = new Set(
[FIELD_NAME_LIST.area, FIELD_NAME_LIST.province, 'address.province'].filter(
Boolean,
),
);

export default function EditHolderFormField({
rule,
contactInformations,
formValues,
onFieldChange,
}: EditHolderFormFieldProps) {
const { t } = useTranslation(['domain', NAMESPACES.COUNTRIES, NAMESPACES.LANGUAGE]);
const { t } = useTranslation([
'domain',
NAMESPACES.COUNTRIES,
NAMESPACES.LANGUAGE,
...Object.values(NAMESPACES.AREA),
]);
const [touched, setTouched] = useState(false);

const isRequired = useMemo((): boolean => {
Expand Down Expand Up @@ -107,19 +118,26 @@ export default function EditHolderFormField({
return 'text';
}, [rule, enumList]);

const fieldSubType = useMemo(
(): 'text' | 'email' | 'number' | 'search' | 'time' | 'password' | 'url' => {
if ([FIELD_NAME_LIST.email].includes(rule.label)) {
return 'email';
}
return 'text';
},
[rule],
);
const fieldSubType = useMemo(():
| 'text'
| 'email'
| 'number'
| 'search'
| 'time'
| 'password'
| 'url' => {
if ([FIELD_NAME_LIST.email].includes(rule.label)) {
return 'email';
}
return 'text';
}, [rule]);

const getEnumTranslationKey = (label: string, value: string): string => {
if (SPECIAL_LABELS.has(label)) {
return `domain_tab_CONTACT_edit_form_enum_${label}_${value}`.replaceAll('.', '_');
return `domain_tab_CONTACT_edit_form_enum_${label}_${value}`.replaceAll(
'.',
'_',
);
}

if (label === FIELD_NAME_LIST.language) {
Expand All @@ -132,27 +150,65 @@ export default function EditHolderFormField({
.replace(/address_country|nationality/, 'country');
}

if (AREA_LABELS.has(label)) {
const country = resolveFormValue(
formValues[FIELD_NAME_LIST.addressCountry],
);
const areaNamespace =
country && NAMESPACES.AREA[country as keyof typeof NAMESPACES.AREA];
if (areaNamespace) {
return `${areaNamespace}:${value}`;
}
}

return `${label}_${value}`;
};

const translatedEnums: TTranslatedEnum[] = useMemo(() => {
return enumList
.map((value) => ({
const isAreaField = AREA_LABELS.has(rule.label);

const translated = enumList.map((value) => {
const translationKey = getEnumTranslationKey(rule.label, value);
const translatedValue = t(translationKey);
const hasTranslation = translatedValue !== translationKey;
return {
key: value,
translated: t(getEnumTranslationKey(rule.label, value)),
}))
.sort((a, b) => a.translated.localeCompare(b.translated));
// For area fields, fall back to the raw value when no translation exists
// (the value is already a readable name like "Antrim").
// For other fields, always use what t() returns (the key itself when untranslated).
translated: isAreaField && !hasTranslation ? value : translatedValue,
hasTranslation,
};
Comment on lines 167 to +181
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

translatedEnums is declared as TTranslatedEnum[], but the mapped objects include an extra hasTranslation property. With the current TTranslatedEnum type (only { key, translated }), this will fail TypeScript’s excess property checks. Consider either extending TTranslatedEnum to include hasTranslation (if it’s part of the domain model), or using a separate local type for this component and ensuring only { key, translated } is passed into form state / onFieldChange.

Copilot uses AI. Check for mistakes.
});

// For area fields, only keep values that have a real translation in the
// translation file. This filters out full-name duplicates (e.g. "Carlow")
// sent by the backend alongside their code counterpart (e.g. "CW").
// Also deduplicate by key since the backend may send the same code twice.
if (isAreaField) {
const seenKeys = new Set<string>();
const seenLabels = new Set<string>();
return translated
.filter((item) => {
if (!item.hasTranslation) return false;
if (seenKeys.has(item.key) || seenLabels.has(item.translated))
return false;
seenKeys.add(item.key);
seenLabels.add(item.translated);
return true;
})
.sort((a, b) => a.translated.localeCompare(b.translated));
}
Comment on lines +184 to +201
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

New behavior was added for province/area enum translation (namespace selection by country, filtering out values without translations, deduplication). EditHolderFormField has an existing test suite, but there are no tests covering this new province/area-specific behavior. Add unit tests to validate: (1) values without translations are filtered for the province field, (2) duplicates are removed, and (3) changing address.country updates the computed translation namespace/options.

Copilot uses AI. Check for mistakes.

return translated.sort((a, b) => a.translated.localeCompare(b.translated));
}, [enumList, rule.label]);
Comment on lines +153 to 204
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

The translatedEnums useMemo depends on t, formValues[FIELD_NAME_LIST.addressCountry] (used to pick the area namespace), and getEnumTranslationKey’s logic, but the dependency array is only [enumList, rule.label]. This can leave province/area options stale when the user changes country and the enum list doesn’t change, and can also prevent recomputing when translations change. Include the relevant dependencies (at least t and the resolved country value) or refactor getEnumTranslationKey into a useCallback with explicit deps.

Copilot uses AI. Check for mistakes.

const labelTranslation = useMemo(() => {
const readOnly = isReadOnly && isRequired;
const translatedLabel = t(
getFieldLabelKey(rule.label),
);
const translatedLabel = t(getFieldLabelKey(rule.label));
return [translatedLabel, ...(readOnly ? ['*'] : [])].join(' ');
}, [rule.label, isReadOnly, isRequired]);
Comment on lines 206 to 210
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

labelTranslation uses t(...) but the useMemo dependency list doesn’t include t (or i18n language/ready state). If the user changes language at runtime, the label can remain in the previous language due to memoization.

Copilot uses AI. Check for mistakes.


// Get current value from formValues
const currentValue = formValues[rule.label];

Expand Down Expand Up @@ -239,11 +295,7 @@ export default function EditHolderFormField({
/>
)}

{validationError && (
<FormFieldError>
{validationError}
</FormFieldError>
)}
{validationError && <FormFieldError>{validationError}</FormFieldError>}
</FormField>
);
}
Loading