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

Fix i18n issue for submit label in delete resource modal #9406

Merged
merged 1 commit into from Jul 6, 2021
Merged

Fix i18n issue for submit label in delete resource modal #9406

merged 1 commit into from Jul 6, 2021

Conversation

rohitkrai03
Copy link
Contributor

Fixes: https://issues.redhat.com/browse/ODC-6103

Analysis / Root cause: Delete resource modal expected an actionLabelKey and directly used it inside t() as a translation key.

Solution Description: Added a prop for sending translated string instead of just key.

Screen shots / Gifs for design review:

Screen.Recording.2021-07-05.at.9.14.38.PM.mov

Browser conformance:

  • Chrome
  • Firefox
  • Safari
  • Edge

@openshift-ci openshift-ci bot added component/helm Related to helm-plugin component/shared Related to console-shared approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 5, 2021
sahil143
sahil143 reviewed Jul 5, 2021
View reviewed changes
Copy link
Contributor

@sahil143 sahil143 left a comment

Choose a reason for hiding this comment

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

Looks good overall.

frontend/packages/console-shared/src/components/modals/DeleteResourceModal.tsx Outdated Show resolved Hide resolved
@invincibleJai
Copy link
Member

/retest

@debsmita1
Copy link
Contributor

verified the changes, and the missing i18n key errors cannot be seen in the console
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 6, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 6, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: debsmita1, rohitkrai03

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-bot
Copy link
Contributor

/retest

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

3 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-merge-robot openshift-merge-robot merged commit 9e7dd02 into openshift:master Jul 6, 2021
@rohitkrai03 rohitkrai03 deleted the odc-6103 branch July 7, 2021 06:00
@spadgett spadgett added this to the v4.9 milestone Jul 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sahil143 sahil143 sahil143 left review comments

@vikram-raj vikram-raj Awaiting requested review from vikram-raj

@zherman0 zherman0 Awaiting requested review from zherman0

Assignees

@debsmita1 debsmita1

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. component/helm Related to helm-plugin component/shared Related to console-shared lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
v4.9
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@rohitkrai03 @invincibleJai @debsmita1 @openshift-bot @sahil143 @spadgett @openshift-merge-robot