patternfly / patternfly-quickstarts

Repository containing the PatternFly Quick Start extension code
MIT License
15 stars 38 forks source link

Adds language and countryCode keys and creates new localization demo #100

Closed gabipodolnikova closed 2 years ago

gabipodolnikova commented 2 years ago

Closes #93

So I didn't put the filtering based on the language inside the library. We are gonna leave it for the consumer, they can filter it how they want it (more flexibility). I added a new demo under /quickstart-localized to demonstrate how it can be done.

netlify[bot] commented 2 years ago

✔️ Deploy Preview for quickstarts ready!

🔨 Explore the source changes: dcfa210b58f18a9d189d9a5f9ee49c99bca1fc9d

🔍 Inspect the deploy log: https://app.netlify.com/sites/quickstarts/deploys/61e8211ad9c94800079d6910

😎 Browse the preview: https://deploy-preview-100--quickstarts.netlify.app/

spadgett commented 2 years ago

Have these changes been released? It would be good to hold off if we can to avoid making a breaking change if we need to change the API to work with custom resources.

jschuler commented 2 years ago

@spadgett that's good to know, yes we should align what fields we want non-OpenShift/k8s users to use for localization purposes to work also with the k8s CRD format. Do you have a schema for it? Looking at the linked proposal, I am seeing

labels:
    console.openshift.io/lang: en
    console.openshift.io/name: explore-serverless

can we change the keys to be more generic and not openshift specific?

Thanks

jschuler commented 2 years ago

Also, it looks like OpenShift does not block from creating a ConsoleQuickStart CRD with those fields

apiVersion: console.openshift.io/v1
kind: ConsoleQuickStart
metadata:
  name: example
  language: en
  country: us
spec:
  displayName: Testing metadata
  tags:
    - example
    - image
    - deployment
  durationMinutes: 5
  description: Testing metadata
  introduction: >-
    ### Hello world
  tasks:
    - title: A new task
      description: >-
        Task description
spadgett commented 2 years ago

OpenShift won't complain, but unknown fields will be dropped when the resource is saved.

spadgett commented 2 years ago

How are fallbacks to en handled? The way the OpenShift proposal works is that if a quick start is translated to the user's preferred language, we use that. Otherwise, we fallback to the English version. So we need some way to know which translations replaces which English version (so we only show one).

The reason we used labels in OpenShift is so that we can use a label selector and only get quick starts back for the languages the user prefers instead of all quick starts from the API.

spadgett commented 2 years ago

can we change the keys to be more generic and not openshift specific?

The API group of the resource is already OpenShift-specific, although I guess PF is ignoring that.

The labels need to be namespaced. We can look at adjusting the labels to domain that is more neutral.

We should discuss if it's good for PF to reuse the OpenShift API in general, however. Any change to the PF resource API will need to go through OpenShift API review, and we have strict rules around API compatibility.