Skip to content
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

Jump to bottom

packages/console-shared localization #6956

Merged
merged 1 commit into from Oct 30, 2020
Merged

packages/console-shared localization #6956

merged 1 commit into from Oct 30, 2020

Conversation

debsmita1
Copy link
Contributor

@debsmita1 debsmita1 commented Oct 19, 2020 •
edited

Story:
https://issues.redhat.com/browse/ODC-4894

Screenshots:
l10
l6
l5
l4
l2
l1
l9
l8

ShortcutsLink component is covered in PR #6889

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. component/core Related to console core functionality labels Oct 19, 2020
@openshift-ci-robot openshift-ci-robot added the component/shared Related to console-shared label Oct 19, 2020
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 20, 2020
@debsmita1 debsmita1 force-pushed the shared-localization branch 2 times, most recently from 42412fc to d9c685e Compare October 21, 2020 16:43
@openshift-ci-robot openshift-ci-robot added component/dev-console Related to dev-console and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 21, 2020
@debsmita1 debsmita1 changed the title [WIP] :packages/console-shared localization packages/console-shared localization Oct 22, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 22, 2020
@debsmita1 debsmita1 force-pushed the shared-localization branch 2 times, most recently from 0029363 to 24c1637 Compare October 22, 2020 20:10
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 22, 2020
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 22, 2020
rottencandy
rottencandy reviewed Oct 23, 2020
View reviewed changes
Copy link
Contributor

@rottencandy rottencandy left a comment •
edited

Choose a reason for hiding this comment

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

Missed the webpack config line:

new CopyWebpackPlugin([{ from: './packages/yourPackageName/locales', to: 'locales' }])

in frontend/webpack.config.ts.
It was already added.

frontend/packages/console-shared/src/components/dynamic-form/fields.tsx Outdated
return (
<div id={`${id}_field`} className="form-group">
{showLabel && label && (
<label className={classnames('form-label', { 'co-required': required })} htmlFor={id}>
{label}
{t('console-shared~{{label}}', { label })}
Copy link
Contributor

Choose a reason for hiding this comment

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

This will have no effect.

frontend/packages/console-shared/src/components/dynamic-form/fields.tsx Outdated
@@ -82,7 +89,7 @@ export const FieldSet: React.FC<FieldSetProps> = ({
className={classnames({ 'co-required': required })}
htmlFor={`${idSchema.$id}_accordion-content`}
>
{_.startCase(label)}
{t('console-shared~{{label}}', { label: _.startCase(label) })}
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

frontend/packages/console-shared/src/components/formik-fields/BaseInputField.tsx Outdated
@@ -37,7 +39,9 @@ const BaseInputField: React.FC<BaseInputFieldProps & {
id: fieldId,
label,
validated: !isValid ? ValidatedOptions.error : validated,
'aria-describedby': `${fieldId}-helper`,
'aria-describedby': t('console-shared~{{ariaDescribed}}', {
Copy link
Contributor

Choose a reason for hiding this comment

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

Here as well.

frontend/packages/console-shared/src/components/formik-fields/DropdownField.tsx Outdated
@@ -30,7 +32,9 @@ const DropdownField: React.FC<DropdownFieldProps> = ({ label, helpText, required
id={fieldId}
selectedKey={field.value}
dropDownClassName={cx({ 'dropdown--full-width': props.fullWidth })}
aria-describedby={`${fieldId}-helper`}
aria-describedby={t('console-shared~{{ariaDescribed}}', {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

frontend/packages/console-shared/src/components/formik-fields/DroppableFileInputField.tsx Outdated
@@ -16,7 +18,9 @@ const DroppableFileInputField: React.FC<FieldProps> = ({ name, label, helpText }
onChange={(fileData: string) => setFieldValue(name, fileData)}
inputFileData={field.value}
inputFieldHelpText={helpText}
aria-describedby={`${fieldId}-helper`}
aria-describedby={t('console-shared~{{ariaDescribed}}', {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same.

frontend/packages/console-shared/src/components/formik-fields/ToggleableFieldBase.tsx Outdated
@@ -40,7 +42,9 @@ const ToggleableFieldBase: React.FC<ToggleableFieldBaseProps> = ({
label,
isChecked: field.checked,
isValid,
'aria-describedby': `${fieldId}-helper`,
'aria-describedby': t('console-shared~{{ariaDescribed}}', {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here. No effect.

frontend/packages/console-shared/src/components/formik-fields/ToggleableFieldBase.tsx Outdated
@@ -40,7 +42,9 @@ const ToggleableFieldBase: React.FC<ToggleableFieldBaseProps> = ({
label,
isChecked: field.checked,
isValid,
'aria-describedby': `${fieldId}-helper`,
'aria-describedby': t('console-shared~{{ariaDescribed}}', {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here. No effect.

frontend/packages/console-shared/src/components/formik-fields/TextAreaField.tsx Outdated
@@ -25,7 +27,9 @@ const TextAreaField: React.FC<TextAreaProps> = ({ label, helpText, required, ...
style={{ resize: 'vertical' }}
validated={isValid ? 'default' : 'error'}
isRequired={required}
aria-describedby={`${fieldId}-helper`}
aria-describedby={t('console-shared~{{ariaDescribed}}', {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

frontend/packages/console-shared/src/components/formik-fields/ResourceLimitField.tsx Outdated
@@ -41,7 +43,7 @@ const ResourceLimitField: React.FC<ResourceLimitFieldProps> = ({
dropdownUnits={unitOptions}
defaultRequestSizeUnit={defaultUnitSize}
defaultRequestSizeValue={field.value}
describedBy={`${fieldId}-helper`}
describedBy={t('console-shared~{{ariaDescribed}}', { ariaDescribed: `${fieldId}-helper` })}
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

frontend/packages/console-shared/src/components/formik-fields/RadioGroupField.tsx Outdated
@@ -50,7 +52,9 @@ const RadioGroupField: React.FC<RadioGroupFieldProps> = ({
value={option.value}
label={option.label}
isDisabled={option.isDisabled}
aria-describedby={`${fieldId}-helper`}
aria-describedby={t('console-shared~{{ariaDescribed}}', {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

@debsmita1 debsmita1 force-pushed the shared-localization branch 2 times, most recently from f6e4070 to 8727cb6 Compare October 23, 2020 09:09
rottencandy
rottencandy approved these changes Oct 23, 2020
View reviewed changes
Copy link
Contributor

@rottencandy rottencandy left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 23, 2020
@debsmita1
Copy link
Contributor Author

/assign @christianvogt

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 24, 2020
@debsmita1
Copy link
Contributor Author

/retest

christianvogt
christianvogt reviewed Oct 28, 2020
View reviewed changes
frontend/packages/console-shared/src/components/dynamic-form/index.tsx Outdated
const schemaErrors = getSchemaErrors(schema);
// IF the top level schema is unsupported, don't render a form at all.
if (schemaErrors.length) {
// eslint-disable-next-line no-console
console.warn('A form could not be generated for this resource.', schemaErrors);
console.warn(
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should be localizing console log messages .

Cc @rebeccaalpert

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want a ruling on this from the accessibility folks @christianvogt? I don't think we've been i18ning console messages in the admin console form what I've worked on/seen, but I don't look at every file the way Sam does.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just wanted to make sure there was no objection to keeping console logs in english. Sounds like we're on the same page.

frontend/packages/console-shared/src/components/formik-fields/RadioGroupField.tsx Outdated
@@ -50,7 +52,7 @@ const RadioGroupField: React.FC<RadioGroupFieldProps> = ({
value={option.value}
label={option.label}
isDisabled={option.isDisabled}
aria-describedby={`${fieldId}-helper`}
aria-describedby={t('console-shared~{{fieldId}}-helper', { fieldId })}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an ID and not text.

frontend/packages/console-shared/src/components/formik-fields/ResourceLimitField.tsx Outdated
@@ -41,7 +43,7 @@ const ResourceLimitField: React.FC<ResourceLimitFieldProps> = ({
dropdownUnits={unitOptions}
defaultRequestSizeUnit={defaultUnitSize}
defaultRequestSizeValue={field.value}
describedBy={`${fieldId}-helper`}
describedBy={t('console-shared~{{fieldId}}-helper', { fieldId })}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an ID and not text.

frontend/packages/console-shared/src/components/formik-fields/TextAreaField.tsx Outdated
@@ -25,7 +27,7 @@ const TextAreaField: React.FC<TextAreaProps> = ({ label, helpText, required, ...
style={{ resize: 'vertical' }}
validated={isValid ? 'default' : 'error'}
isRequired={required}
aria-describedby={`${fieldId}-helper`}
aria-describedby={t('console-shared~{{fieldId}}-helper', { fieldId })}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an ID and not text.

frontend/packages/console-shared/src/components/formik-fields/ToggleableFieldBase.tsx Outdated
@@ -40,7 +42,7 @@ const ToggleableFieldBase: React.FC<ToggleableFieldBaseProps> = ({
label,
isChecked: field.checked,
isValid,
'aria-describedby': `${fieldId}-helper`,
'aria-describedby': t('console-shared~{{fieldId}}-helper', { fieldId }),
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an ID and not text.

frontend/packages/console-shared/src/components/popover/Popover.tsx Outdated
<PopoverContent>
<PopoverCloseButton
onClose={onClose}
aria-label={t('console-shared~closeBtnAriaLabel')}
Copy link
Contributor

Choose a reason for hiding this comment

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

Set to Close

frontend/packages/console-shared/src/utils/pod-ring-utils.ts Outdated
podRingLabelData.reversed = true;
break;
}
if (isPending) {
podRingLabelData.title = '0';
podRingLabelData.subTitle = `scaling to ${desiredPodCount}`;
podRingLabelData.title = t('console-shared~0');
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to localize single digit numbers.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 28, 2020
debsmita1
debsmita1 commented Oct 28, 2020
View reviewed changes
frontend/public/i18n.js
@@ -85,11 +85,11 @@ i18n
'utils',
'webhook-receiver-form',
'yaml',
'editor',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed this because it is already added in line no: 55

@debsmita1 debsmita1 force-pushed the shared-localization branch 2 times, most recently from a8214ca to f93f96a Compare October 29, 2020 06:08
@christianvogt
Copy link
Contributor

/approve
/lgtm

@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 29, 2020
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

7 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 30, 2020
@debsmita1
Copy link
Contributor Author

rebased

rottencandy
rottencandy approved these changes Oct 30, 2020
View reviewed changes
Copy link
Contributor

@rottencandy rottencandy left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 30, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: christianvogt, debsmita1, rottencandy

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit 8e2a3d0 into openshift:master Oct 30, 2020
@spadgett spadgett added this to the v4.7 milestone Nov 4, 2020
@debsmita1
Copy link
Contributor Author

/kind feature

@openshift-ci-robot openshift-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jan 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rebeccaalpert rebeccaalpert rebeccaalpert left review comments

@christianvogt christianvogt christianvogt left review comments

@rottencandy rottencandy rottencandy approved these changes

@dtaylor113 dtaylor113 Awaiting requested review from dtaylor113

@zherman0 zherman0 Awaiting requested review from zherman0

Assignees

@christianvogt christianvogt

@rottencandy rottencandy

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. component/core Related to console core functionality component/dev-console Related to dev-console component/shared Related to console-shared kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
v4.7
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@debsmita1 @christianvogt @openshift-bot @openshift-ci-robot @rebeccaalpert @rottencandy @spadgett @openshift-merge-robot