CONSOLE-4626: Creating YAML/Form view perspective switcher for Creating CronTabs by krishagarwal278 · Pull Request #34 · openshift/console-crontab-plugin

krishagarwal278 · 2025-07-02T21:12:09Z

Added switcher
working on YAML Editor

openshift-ci-robot · 2025-07-02T21:12:13Z

@krishagarwal278: This pull request references CONSOLE-4626 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set.

Details

In response to this:

Added switcher
working on YAML Editor

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 openshift-eng/jira-lifecycle-plugin repository.

jhadvig

@krishagarwal278 I would suggest to follow the simple pattern we are doing in PVC create form, where we are only PageHeading with simple link to the code editor https://github.com/openshift/console/blob/main/frontend/public/components/storage/create-pvc.tsx#L265-L273
I would avoid any migration between the form and the editor.

rhamilto · 2025-07-11T12:11:48Z

src/views/CronTabForm/CronTabForm.tsx

-    <PageSection isFilled={true} height="sizeToFit">
+    <>
+      <div data-test="page-heading">
+        <PageHeader title={t("Create CronTab")} data-test="page-heading" />


Suggested change

<PageHeader title={t("Create CronTab")} data-test="page-heading" />

<PageHeader title={t("Create CronTab")} data-test="page-heading" />

Suggested change

<PageHeader title={t("Create CronTab")} data-test="page-heading" />

<PageHeader title={t("Create CronTab")} />

krishagarwal278 · 2025-07-11T16:02:16Z

/retest

krishagarwal278 · 2025-07-11T17:08:46Z

/test e2e-aws-console-crontab-plugin

krishagarwal278 · 2025-07-11T19:46:33Z

/retest

krishagarwal278 · 2025-07-11T21:58:12Z

/retest

krishagarwal278 · 2025-07-12T15:28:37Z

/retest

krishagarwal278 · 2025-07-13T00:21:47Z

/retest

krishagarwal278 · 2025-07-14T20:23:51Z

/retest

krishagarwal278 · 2025-07-16T14:24:46Z

/retest

rhamilto

A few comments.

src/views/CronTabForm/CronTabForm.tsx

rhamilto

Nice work on this @krishagarwal278. Reviewing this again made me realize we've made a few concessions that we should reconsider. Let me know if you'd like to discuss tomorrow.

rhamilto · 2025-07-16T20:44:37Z

locales/en/plugin__console-crontab-plugin.json

-}
+  "The desired number of instances of this CronTab to run.": "The desired number of instances of this CronTab to run.",
+  "YAML Editor": "YAML Editor",
+  "YAML must include metadata.name or metadata.generateName": "YAML must include metadata.name or metadata.generateName",


You need to run yarn i18n again and commit the changes since you removed these strings.

rhamilto · 2025-07-16T21:07:22Z

src/views/CronTabForm/CronTabForm.tsx

+      <PageSection isFilled={true} height="sizeToFit">
+        {showYaml ? (
+          <>
+            <CodeEditor


This has me realizing we won't have full feature parity with the default code editor in the console with the sidebar and other options, which is a limitation of <CodeEditor>.

What we should probably do is revert to what we had before you implemented the form as that will get us the default console editor for creation and add the form with the ability to toggle from the form to the default editor.

So /k8s/ns/*/stable.example.com~v1~CronTab/~new would be the default editor and /k8s/ns/*/stable.example.com~v1~CronTab/~new/form (a new route) would be the form, the header of which would have a link to the default editor.

console-crontab-plugin/src/views/CronTabList/CronTabList.tsx

Line 76 in 70d2d4f

}~${cronTabGroupVersionKind.version}~${cronTabGroupVersionKind.kind}/~new`;

would change to }~${cronTabGroupVersionKind.version}~${cronTabGroupVersionKind.kind}/~new/form; so that the form is the default with a link back to the editor.

StorageClasses is an example

Screen.Recording.2025-07-16.at.5.29.05.PM.mov

rhamilto

Looking good, @krishagarwal278. One error and and two nits.

rhamilto · 2025-07-17T20:17:23Z

src/views/CronTabForm/CronTabForm.tsx

  const [replicas, setReplicas] = useState<number | "">(0);
  const [loading, setLoading] = useState(false);
  const [error, setError] = useState("");
+


Nit: no need for an extra line since theses are all hooks.

rhamilto · 2025-07-17T20:18:15Z

console-extensions.json

      "path": "spec.cronSpec"
    }
  },
+  {


Nit: to make the diff a little easer to follow, move this to line 65 so it's more clear you are replacing console.resource/create with this.

rhamilto · 2025-07-17T20:18:50Z

src/views/CronTabList/CronTabList.tsx

  const createAccessReview = {
    groupVersionKind: cronTabGroupVersionKind,
-    namespace: namespace || "default",
+    namespace: namespace || "form",


Suggested change

namespace: namespace || "form",

namespace: namespace || "default",

rhamilto · 2025-07-17T20:58:16Z

console-extensions.json

-      "flags": {
-        "required": ["CRONTAB"]
-      }
+      "exact": true


This is where it's good to have the side-by-side comparison!

This addition should really be protected by a flag. But note #35. The existing flags are in the wrong location, so they don't work. So let's merge this as is and I will add the flag in #35.

rhamilto · 2025-07-18T12:42:47Z

just curious, why we don't have a Show/Hide Sidebar button on CronTab YAML page?

Good catch, @yapei. I am guessing this is the result of a regression introduced with openshift/console#15254. @logonoff, mind taking a look?

rhamilto · 2025-07-18T12:50:27Z

just curious, why we don't have a Show/Hide Sidebar button on CronTab YAML page?

Good catch, @yapei. I am guessing this is the result of a regression introduced with openshift/console#15254. @logonoff, mind taking a look?

Hmm. This is working correctly when I run the plugin locally.

opayne1 · 2025-07-18T13:31:15Z

/label docs-approved

rhamilto · 2025-07-18T15:35:14Z

just curious, why we don't have a Show/Hide Sidebar button on CronTab YAML page?

Good catch, @yapei. I am guessing this is the result of a regression introduced with openshift/console#15254. @logonoff, mind taking a look?

Hmm. This is working correctly when I run the plugin locally.

And it is also working if I deploy this PR to my shared cluster:

console-openshift-console apps rhamilto devcluster openshift com_dashboards

rhamilto · 2025-07-18T17:14:54Z

/retest

krishagarwal278 · 2025-07-18T17:34:00Z

Helloo @rhamilto, this video, demonstrates the changes we made. have not shown the crontab being deleted or edited in this video as it was not in scope of the story. Really gratefuly your contribution and guidance! :)))

Screen.Recording.2025-07-18.at.1.29.07.PM.mov

sferich888 · 2025-07-18T19:50:01Z

/label px-approved

yapei · 2025-07-21T02:51:50Z

sorry I tested again, this time I see Show/Hide sidebar button

/label qe-approved

openshift-ci-robot · 2025-07-21T02:52:07Z

@krishagarwal278: This pull request references CONSOLE-4626 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set.

Details

In response to this:

Added switcher
working on YAML Editor

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 openshift-eng/jira-lifecycle-plugin repository.

logonoff · 2025-07-21T12:08:11Z

src/views/CronTabForm/CronTabForm.tsx

+      <div data-test="page-heading">
+        <PageHeader
+          title={t("Create CronTab")}
+          linkProps={{
+            label: t("Edit YAML"),
+            onClick: (e) => {
+              e.preventDefault();
+              navigate(
+                `/k8s/ns/${namespace}/${cronTabGroupVersionKind.group}~${cronTabGroupVersionKind.version}~${cronTabGroupVersionKind.kind}/~new`
+              );
+            },
+          }}
+        />
+      </div>


can we use ListPageHeader from the SDK so that you can favourite the page

ListPageHeader doesn't have a link prop that we can use to redirect user to YAML view. This link is soon going to be replaced with a component that's exposed to the SDK so all original functionality like favoriting the page can be preserved

@krishagarwal278, you should be able to use helpText to pass in the link. Let me know if it does not work. I realize that prop name does not match your exact use case based on the name of the prop, but it's ok.

krishagarwal278 · 2025-07-21T14:22:38Z

Screen.Recording.2025-07-21.at.10.18.04.AM.mov

Replacing PageHeader with ListPageHeader allows to favorite the page

rhamilto · 2025-07-21T14:23:02Z

/lgtm

logonoff

/lgtm

openshift-ci · 2025-07-21T14:34:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: krishagarwal278, logonoff, rhamilto

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:

~~OWNERS~~ [rhamilto]

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

openshift-ci · 2025-07-21T15:47:48Z

@krishagarwal278: 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-sigs/prow repository. I understand the commands that are listed here.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 2, 2025

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 2, 2025

openshift-ci bot requested review from TheRealJon and jhadvig July 2, 2025 21:12

WIP: Getting error when editing YAML on clicking Link

66edc78

krishagarwal278 force-pushed the CONSOLE-4626 branch from aeb54cf to 66edc78 Compare July 7, 2025 17:54

krishagarwal278 changed the title ~~[WIP] CONSOLE-4626: Creating YAML/Form view perspective switcher for Creating CronTabs~~ CONSOLE-4626: Creating YAML/Form view perspective switcher for Creating CronTabs Jul 7, 2025

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 7, 2025

jhadvig reviewed Jul 8, 2025

View reviewed changes

Added perspective switcher with YAML Editor

e4d40b8

krishagarwal278 force-pushed the CONSOLE-4626 branch from 3e26a21 to e4d40b8 Compare July 9, 2025 21:08

rhamilto reviewed Jul 11, 2025

View reviewed changes

krishagarwal278 force-pushed the CONSOLE-4626 branch from 18a4cc1 to 09c9cbd Compare July 11, 2025 13:51

rhamilto reviewed Jul 16, 2025

View reviewed changes

src/views/CronTabForm/CronTabForm.tsx Outdated Show resolved Hide resolved

src/views/CronTabForm/CronTabForm.tsx Outdated Show resolved Hide resolved

src/views/CronTabForm/CronTabForm.tsx Outdated Show resolved Hide resolved

krishagarwal278 force-pushed the CONSOLE-4626 branch from 09c9cbd to b7e77b0 Compare July 16, 2025 18:04

rhamilto reviewed Jul 16, 2025

View reviewed changes

krishagarwal278 force-pushed the CONSOLE-4626 branch from b7e77b0 to 4403b67 Compare July 17, 2025 20:06

rhamilto reviewed Jul 17, 2025

View reviewed changes

Changed Code Editor to YAML editor by changing console extensions

ea3fdd8

krishagarwal278 force-pushed the CONSOLE-4626 branch from 4403b67 to ea3fdd8 Compare July 17, 2025 20:46

rhamilto reviewed Jul 17, 2025

View reviewed changes

openshift-ci bot added the docs-approved Signifies that Docs has signed off on this PR label Jul 18, 2025

openshift-ci bot added the px-approved Signifies that Product Support has signed off on this PR label Jul 18, 2025

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 21, 2025

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Jul 21, 2025

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 21, 2025

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 21, 2025

logonoff reviewed Jul 21, 2025

View reviewed changes

Replaced PageHeader with ListPageHeader for favorites modal

b619087

krishagarwal278 force-pushed the CONSOLE-4626 branch from 1e244c2 to b619087 Compare July 21, 2025 14:15

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 21, 2025

Merge branch 'main' into CONSOLE-4626

23ce57a

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 21, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 21, 2025

logonoff approved these changes Jul 21, 2025

View reviewed changes

openshift-ci bot assigned logonoff Jul 21, 2025

openshift-merge-bot bot merged commit 8c37c13 into openshift:main Jul 21, 2025
4 checks passed

krishagarwal278 deleted the CONSOLE-4626 branch October 14, 2025 14:53

	<PageHeader title={t("Create CronTab")} data-test="page-heading" />
	<PageHeader title={t("Create CronTab")} data-test="page-heading" />

	namespace: namespace \|\| "form",
	namespace: namespace \|\| "default",

Conversation

krishagarwal278 commented Jul 2, 2025

Uh oh!

openshift-ci-robot commented Jul 2, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jhadvig left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krishagarwal278 commented Jul 11, 2025

Uh oh!

krishagarwal278 commented Jul 11, 2025

Uh oh!

krishagarwal278 commented Jul 11, 2025

Uh oh!

krishagarwal278 commented Jul 11, 2025

Uh oh!

krishagarwal278 commented Jul 12, 2025

Uh oh!

krishagarwal278 commented Jul 13, 2025

Uh oh!

krishagarwal278 commented Jul 14, 2025

Uh oh!

krishagarwal278 commented Jul 16, 2025

Uh oh!

rhamilto left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rhamilto left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rhamilto left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rhamilto commented Jul 18, 2025

Uh oh!

rhamilto commented Jul 18, 2025

Uh oh!

opayne1 commented Jul 18, 2025

Uh oh!

rhamilto commented Jul 18, 2025

Uh oh!

rhamilto commented Jul 18, 2025

Uh oh!

krishagarwal278 commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sferich888 commented Jul 18, 2025

Uh oh!

yapei commented Jul 21, 2025

Uh oh!

openshift-ci-robot commented Jul 21, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krishagarwal278 commented Jul 21, 2025

Uh oh!

openshift-ci-robot commented Jul 2, 2025 •

edited by openshift-ci bot

Loading

jhadvig left a comment •

edited

Loading

krishagarwal278 commented Jul 18, 2025 •

edited

Loading

openshift-ci-robot commented Jul 21, 2025 •

edited by openshift-ci bot

Loading