Skip to content

Conversation

@christoph-jerolimov
Copy link
Member

@christoph-jerolimov christoph-jerolimov commented Oct 14, 2022

Fixes:
https://issues.redhat.com/browse/OCPBUGS-2430

Analysis / Root cause:
@invincibleJai reported in the customization form code review #12159 (comment) that nothing is rendered when no Quick Start is found or all Quick Starts are disabled. While debugging I noticed that the PatternFly component handles this already:

https://github.com/patternfly/patternfly-quickstarts/blob/e7ddc87bb040733029c0f5be7873542b41d4871e/packages/module/src/QuickStartCatalogPage.tsx#L156-L158

https://github.com/patternfly/patternfly-quickstarts/blob/e7ddc87bb040733029c0f5be7873542b41d4871e/packages/module/src/ConsoleInternal/components/utils/status-box.tsx#L26-L38

But it doesn't render anything. The underlying issue is that the translations for "No {{label}} found" was found. PF QuickStarts doesn't show anything in this case. I opened a ticket that this should return the not translated version in that case: patternfly/patternfly-quickstarts#194

Solution Description:
To fix this on our side, and also to translate these values I've added some missed Quick Start strings.

Just searched for getResource calls in https://github.com/patternfly/patternfly-quickstarts

Screenshots:

Before when no Quick Starts were found:

master-en-no-quickstarts

With this PR when no Quick Starts were found:

pr-en-no-quickstart

Other languages will not show anything until the translation is added or patternfly/patternfly-quickstarts#194 is fixed/merged.

Before with missing translation in the sidebar:

master-pseudo

master-zh-missing-translation

With this PR:

pr-pseudo

No Quick start flicker with new translation, but without changes at the loaded state:

without-ts-change.mp4

With updated loading/loaded state:

with-ts-change.mp4

Unit test coverage report:
Unchanged

Test setup:
Disable all quick starts, on a local bridge you need to create the config.yaml manually and start the bridge like this:

bin/bridge -config ../config.yaml
apiVersion: console.openshift.io/v1
kind: ConsoleConfig
customization:
  quickStarts:
    disabled:
      - "quarkus-with-s2i"
      - "spring-with-s2i"
      - "monitor-sampleapp"
      - "install-app-and-associate-pipeline"
      - "odf-install-tour"
      - "quarkus-with-helm"
      - "sample-application"
      - "node-with-s2i"
      - "install-serverless"
      - "install-helmchartrepo-ns"
      - "add-healthchecks"
      - "explore-pipelines"
      - "manage-helm-repos"
      - "configure-pipeline-metrics"

Browser conformance:

  • Chrome
  • Firefox
  • Safari
  • Edge

@openshift-ci openshift-ci bot requested review from cajieh and kyoto October 14, 2022 22:56
@openshift-ci openshift-ci bot added component/core Related to console core functionality kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 14, 2022
@christoph-jerolimov christoph-jerolimov force-pushed the missing-quickstart-translations branch from a42c87f to a4c4546 Compare October 14, 2022 23:43
@christoph-jerolimov christoph-jerolimov changed the title Add missed Quick Start translation OCPBUGS-2430: Add missing Quick Start translation Oct 14, 2022
@openshift-ci-robot openshift-ci-robot added jira/severity-low Referenced Jira bug's severity is low for the branch this PR is targeting. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Oct 14, 2022
@openshift-ci-robot
Copy link
Contributor

@jerolimov: This pull request references Jira Issue OCPBUGS-2430, which is invalid:

  • expected the bug to target only the "4.12.0" version, but multiple target versions were set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Fixes:
todo

Analysis / Root cause:
@invincibleJai reported in the customization form code review #12159 (comment) that nothing is rendered when no Quick Start is found or all Quick Starts are disabled. While debugging I noticed that the PatternFly component handles this already:

https://github.com/patternfly/patternfly-quickstarts/blob/e7ddc87bb040733029c0f5be7873542b41d4871e/packages/module/src/QuickStartCatalogPage.tsx#L156-L158

https://github.com/patternfly/patternfly-quickstarts/blob/e7ddc87bb040733029c0f5be7873542b41d4871e/packages/module/src/ConsoleInternal/components/utils/status-box.tsx#L26-L38

But it doesn't render anything. The underlying issue is that the translations for "No {{label}} found" was found. PF QuickStarts doesn't show anything in this case. I opened a ticket that this should return the not translated version in that case: patternfly/patternfly-quickstarts#194

Solution Description:
To fix this on our side, and also to translate these values I've added some missed Quick Start strings.

Just searched for getResource calls in https://github.com/patternfly/patternfly-quickstarts

Screenshots:

Before:

After:

Unit test coverage report:
Unchanged

Test setup:
Disable all quick starts, on a local bridge you need to create the config.yaml manually and start the bridge like this:

bin/bridge -config ../config.yaml
apiVersion: console.openshift.io/v1
kind: ConsoleConfig
 quickStarts:
   disabled:
     - "quarkus-with-s2i"
     - "spring-with-s2i"
     - "monitor-sampleapp"
     - "install-app-and-associate-pipeline"
     - "odf-install-tour"
     - "quarkus-with-helm"
     - "sample-application"
     - "node-with-s2i"
     - "install-serverless"
     - "install-helmchartrepo-ns"
     - "add-healthchecks"
     - "explore-pipelines"
     - "manage-helm-repos"
     - "configure-pipeline-metrics"

Browser conformance:

  • Chrome
  • Firefox
  • Safari
  • Edge

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot
Copy link
Contributor

@jerolimov: This pull request references Jira Issue OCPBUGS-2430, which is invalid:

  • expected the bug to target only the "4.12.0" version, but multiple target versions were set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

Fixes:
https://issues.redhat.com/browse/OCPBUGS-2430

Analysis / Root cause:
@invincibleJai reported in the customization form code review #12159 (comment) that nothing is rendered when no Quick Start is found or all Quick Starts are disabled. While debugging I noticed that the PatternFly component handles this already:

https://github.com/patternfly/patternfly-quickstarts/blob/e7ddc87bb040733029c0f5be7873542b41d4871e/packages/module/src/QuickStartCatalogPage.tsx#L156-L158

https://github.com/patternfly/patternfly-quickstarts/blob/e7ddc87bb040733029c0f5be7873542b41d4871e/packages/module/src/ConsoleInternal/components/utils/status-box.tsx#L26-L38

But it doesn't render anything. The underlying issue is that the translations for "No {{label}} found" was found. PF QuickStarts doesn't show anything in this case. I opened a ticket that this should return the not translated version in that case: patternfly/patternfly-quickstarts#194

Solution Description:
To fix this on our side, and also to translate these values I've added some missed Quick Start strings.

Just searched for getResource calls in https://github.com/patternfly/patternfly-quickstarts

Screenshots:

Before when no Quick Starts were found:

master-en-no-quickstarts

With this PR when no Quick Starts were found:

pr-en-no-quickstart

Before with missing translation in the sidebar:

master-pseudo

master-zh-missing-translation

With this PR:

pr-pseudo

No Quick start flicker with new translation, but without changes at the loaded state:

without-ts-change.mp4

With updated loading/loaded state:

with-ts-change.mp4

Unit test coverage report:
Unchanged

Test setup:
Disable all quick starts, on a local bridge you need to create the config.yaml manually and start the bridge like this:

bin/bridge -config ../config.yaml
apiVersion: console.openshift.io/v1
kind: ConsoleConfig
 quickStarts:
   disabled:
     - "quarkus-with-s2i"
     - "spring-with-s2i"
     - "monitor-sampleapp"
     - "install-app-and-associate-pipeline"
     - "odf-install-tour"
     - "quarkus-with-helm"
     - "sample-application"
     - "node-with-s2i"
     - "install-serverless"
     - "install-helmchartrepo-ns"
     - "add-healthchecks"
     - "explore-pipelines"
     - "manage-helm-repos"
     - "configure-pipeline-metrics"

Browser conformance:

  • Chrome
  • Firefox
  • Safari
  • Edge

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@christoph-jerolimov
Copy link
Member Author

/jira refresh
/kind bug
/uncc kyoto cyril-ui-developer
/cc @invincibleJai @debsmita1

@openshift-ci openshift-ci bot removed request for cajieh and kyoto October 14, 2022 23:59
@openshift-ci-robot openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Oct 14, 2022
@openshift-ci-robot openshift-ci-robot removed the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Oct 15, 2022
@openshift-ci-robot
Copy link
Contributor

@jerolimov: This pull request references Jira Issue OCPBUGS-2430, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.12.0) matches configured target version for branch (4.12.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)
Details

In response to this:

/jira refresh
/kind bug
/uncc kyoto cyril-ui-developer
/cc @invincibleJai @debsmita1

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci openshift-ci bot added the kind/bug Categorizes issue or PR as related to a bug. label Oct 15, 2022
@openshift-ci-robot
Copy link
Contributor

@jerolimov: This pull request references Jira Issue OCPBUGS-2430, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.12.0) matches configured target version for branch (4.12.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)
Details

In response to this:

Fixes:
https://issues.redhat.com/browse/OCPBUGS-2430

Analysis / Root cause:
@invincibleJai reported in the customization form code review #12159 (comment) that nothing is rendered when no Quick Start is found or all Quick Starts are disabled. While debugging I noticed that the PatternFly component handles this already:

https://github.com/patternfly/patternfly-quickstarts/blob/e7ddc87bb040733029c0f5be7873542b41d4871e/packages/module/src/QuickStartCatalogPage.tsx#L156-L158

https://github.com/patternfly/patternfly-quickstarts/blob/e7ddc87bb040733029c0f5be7873542b41d4871e/packages/module/src/ConsoleInternal/components/utils/status-box.tsx#L26-L38

But it doesn't render anything. The underlying issue is that the translations for "No {{label}} found" was found. PF QuickStarts doesn't show anything in this case. I opened a ticket that this should return the not translated version in that case: patternfly/patternfly-quickstarts#194

Solution Description:
To fix this on our side, and also to translate these values I've added some missed Quick Start strings.

Just searched for getResource calls in https://github.com/patternfly/patternfly-quickstarts

Screenshots:

Before when no Quick Starts were found:

master-en-no-quickstarts

With this PR when no Quick Starts were found:

pr-en-no-quickstart

Other languages will not show anything until the translation is added or patternfly/patternfly-quickstarts#194 is fixed/merged.

Before with missing translation in the sidebar:

master-pseudo

master-zh-missing-translation

With this PR:

pr-pseudo

No Quick start flicker with new translation, but without changes at the loaded state:

without-ts-change.mp4

With updated loading/loaded state:

with-ts-change.mp4

Unit test coverage report:
Unchanged

Test setup:
Disable all quick starts, on a local bridge you need to create the config.yaml manually and start the bridge like this:

bin/bridge -config ../config.yaml
apiVersion: console.openshift.io/v1
kind: ConsoleConfig
 quickStarts:
   disabled:
     - "quarkus-with-s2i"
     - "spring-with-s2i"
     - "monitor-sampleapp"
     - "install-app-and-associate-pipeline"
     - "odf-install-tour"
     - "quarkus-with-helm"
     - "sample-application"
     - "node-with-s2i"
     - "install-serverless"
     - "install-helmchartrepo-ns"
     - "add-healthchecks"
     - "explore-pipelines"
     - "manage-helm-repos"
     - "configure-pipeline-metrics"

Browser conformance:

  • Chrome
  • Firefox
  • Safari
  • Edge

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot
Copy link
Contributor

@jerolimov: This pull request references Jira Issue OCPBUGS-2430, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.12.0) matches configured target version for branch (4.12.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)
Details

In response to this:

Fixes:
https://issues.redhat.com/browse/OCPBUGS-2430

Analysis / Root cause:
@invincibleJai reported in the customization form code review #12159 (comment) that nothing is rendered when no Quick Start is found or all Quick Starts are disabled. While debugging I noticed that the PatternFly component handles this already:

https://github.com/patternfly/patternfly-quickstarts/blob/e7ddc87bb040733029c0f5be7873542b41d4871e/packages/module/src/QuickStartCatalogPage.tsx#L156-L158

https://github.com/patternfly/patternfly-quickstarts/blob/e7ddc87bb040733029c0f5be7873542b41d4871e/packages/module/src/ConsoleInternal/components/utils/status-box.tsx#L26-L38

But it doesn't render anything. The underlying issue is that the translations for "No {{label}} found" was found. PF QuickStarts doesn't show anything in this case. I opened a ticket that this should return the not translated version in that case: patternfly/patternfly-quickstarts#194

Solution Description:
To fix this on our side, and also to translate these values I've added some missed Quick Start strings.

Just searched for getResource calls in https://github.com/patternfly/patternfly-quickstarts

Screenshots:

Before when no Quick Starts were found:

master-en-no-quickstarts

With this PR when no Quick Starts were found:

pr-en-no-quickstart

Other languages will not show anything until the translation is added or patternfly/patternfly-quickstarts#194 is fixed/merged.

Before with missing translation in the sidebar:

master-pseudo

master-zh-missing-translation

With this PR:

pr-pseudo

No Quick start flicker with new translation, but without changes at the loaded state:

without-ts-change.mp4

With updated loading/loaded state:

with-ts-change.mp4

Unit test coverage report:
Unchanged

Test setup:
Disable all quick starts, on a local bridge you need to create the config.yaml manually and start the bridge like this:

bin/bridge -config ../config.yaml
apiVersion: console.openshift.io/v1
kind: ConsoleConfig
customization:
 quickStarts:
   disabled:
     - "quarkus-with-s2i"
     - "spring-with-s2i"
     - "monitor-sampleapp"
     - "install-app-and-associate-pipeline"
     - "odf-install-tour"
     - "quarkus-with-helm"
     - "sample-application"
     - "node-with-s2i"
     - "install-serverless"
     - "install-helmchartrepo-ns"
     - "add-healthchecks"
     - "explore-pipelines"
     - "manage-helm-repos"
     - "configure-pipeline-metrics"

Browser conformance:

  • Chrome
  • Firefox
  • Safari
  • Edge

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@christoph-jerolimov
Copy link
Member Author

/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 17, 2022

@jerolimov: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@invincibleJai
Copy link
Member

Thanks @jerolimov this looks good,

would have liked to see empty state with page heading but i see here in pf it just returns EmptyBox https://github.com/patternfly/patternfly-quickstarts/blob/c00e128724d4ae31f6ee22bf369810f27f929243/packages/module/src/QuickStartCatalogPage.tsx#L156-L158

@invincibleJai
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 18, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 18, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: invincibleJai, jerolimov

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

The pull request process is described here

Details 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 728a410 into openshift:master Oct 18, 2022
@openshift-ci-robot
Copy link
Contributor

@jerolimov: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-2430 has been moved to the MODIFIED state.

Details

In response to this:

Fixes:
https://issues.redhat.com/browse/OCPBUGS-2430

Analysis / Root cause:
@invincibleJai reported in the customization form code review #12159 (comment) that nothing is rendered when no Quick Start is found or all Quick Starts are disabled. While debugging I noticed that the PatternFly component handles this already:

https://github.com/patternfly/patternfly-quickstarts/blob/e7ddc87bb040733029c0f5be7873542b41d4871e/packages/module/src/QuickStartCatalogPage.tsx#L156-L158

https://github.com/patternfly/patternfly-quickstarts/blob/e7ddc87bb040733029c0f5be7873542b41d4871e/packages/module/src/ConsoleInternal/components/utils/status-box.tsx#L26-L38

But it doesn't render anything. The underlying issue is that the translations for "No {{label}} found" was found. PF QuickStarts doesn't show anything in this case. I opened a ticket that this should return the not translated version in that case: patternfly/patternfly-quickstarts#194

Solution Description:
To fix this on our side, and also to translate these values I've added some missed Quick Start strings.

Just searched for getResource calls in https://github.com/patternfly/patternfly-quickstarts

Screenshots:

Before when no Quick Starts were found:

master-en-no-quickstarts

With this PR when no Quick Starts were found:

pr-en-no-quickstart

Other languages will not show anything until the translation is added or patternfly/patternfly-quickstarts#194 is fixed/merged.

Before with missing translation in the sidebar:

master-pseudo

master-zh-missing-translation

With this PR:

pr-pseudo

No Quick start flicker with new translation, but without changes at the loaded state:

without-ts-change.mp4

With updated loading/loaded state:

with-ts-change.mp4

Unit test coverage report:
Unchanged

Test setup:
Disable all quick starts, on a local bridge you need to create the config.yaml manually and start the bridge like this:

bin/bridge -config ../config.yaml
apiVersion: console.openshift.io/v1
kind: ConsoleConfig
customization:
 quickStarts:
   disabled:
     - "quarkus-with-s2i"
     - "spring-with-s2i"
     - "monitor-sampleapp"
     - "install-app-and-associate-pipeline"
     - "odf-install-tour"
     - "quarkus-with-helm"
     - "sample-application"
     - "node-with-s2i"
     - "install-serverless"
     - "install-helmchartrepo-ns"
     - "add-healthchecks"
     - "explore-pipelines"
     - "manage-helm-repos"
     - "configure-pipeline-metrics"

Browser conformance:

  • Chrome
  • Firefox
  • Safari
  • Edge

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@christoph-jerolimov christoph-jerolimov deleted the missing-quickstart-translations branch August 7, 2023 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. component/core Related to console core functionality jira/severity-low Referenced Jira bug's severity is low for the branch this PR is targeting. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. kind/bug Categorizes issue or PR as related to a bug. kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants