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.

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

[Scaffolder] Review and create didn't hide when change options #12948

Open
tritri251214 opened this issue Aug 3, 2022 · 20 comments · May be fixed by #24826
Open

[Scaffolder] Review and create didn't hide when change options #12948

tritri251214 opened this issue Aug 3, 2022 · 20 comments · May be fixed by #24826
Labels
area:scaffolder Everything and all things related to the scaffolder project area bug Something isn't working help wanted Help/Contributions wanted from community members

Comments

@tritri251214
Copy link
Contributor

bandicam.2022-08-03.15-04-51-966.mp4
apiVersion: scaffolder.backstage.io/v1beta3
kind: Template
metadata:
  name: custom-field-2-example
  title: Custom Field 2 Template
  description: An example template for the scaffolder that creates a simple Node.js service
spec:
  owner: user:guest
  type: service
  parameters:
    - title: Fill in some steps
      required:
        - name
      properties:
        name:
          title: Name
          type: string
          description: Unique name of the component
          ui:autofocus: true
          ui:options:
            rows: 5
        generation:
          type: string
          title: Generation Method
          enum:
            - Option 1
            - Option 2
          default: Option 1
      dependencies:
        generation:
          oneOf:
            - properties:
                generation:
                  const: Option 1
            - required:
                - customField
              properties:
                generation:
                  const: Option 2
                customField:
                  title: Custom Field
                  type: string
                  ui:field: CustomField

    - title: Choose a location
      required:
        - repoUrl
      properties:
        repoUrl:
          title: Repository Location
          type: string
          ui:field: RepoUrlPicker
          ui:options:
            allowedHosts:
              - github.com

  steps:
    - id: fetch-base
      name: Fetch Base
      action: fetch:template
      input:
        url: ./content
        values:
          name: ${{ parameters.name }}

    - id: publish
      name: Publish
      action: publish:github
      input:
        allowedHosts: ['github.com']
        description: This is ${{ parameters.name }}
        repoUrl: ${{ parameters.repoUrl }}

    - id: register
      name: Register
      action: catalog:register
      input:
        repoContentsUrl: ${{ steps.publish.output.repoContentsUrl }}
        catalogInfoPath: '/catalog-info.yaml'

  output:
    links:
      - title: Repository
        url: ${{ steps.publish.output.remoteUrl }}
      - title: Open in catalog
        icon: catalog
        entityRef: ${{ steps.register.output.entityRef }}

Custom Field Extension file:

image

Expected Behavior

Hide Custom Field in Review and create

Actual Behavior

Custom Field displayed in Review and create

Steps to Reproduce

  1. Input Name
  2. Select Generation Method is Option 2
  3. Input Custom Field
  4. Click button Next
  5. Input RepoUrl field
  6. Click button Next => Displayed value of Custom Field in Review and create
  7. Click button Back
  8. Click button Back => => Back to Step Fill in some steps
  9. Select Generation Method is Option 1
  10. Click button Next
  11. Click button Next => Custom Field still display in Review and create

Context

Your Environment

  • Browser Information:

  • Output of yarn backstage-cli info:

OS:   Windows_NT 10.0.18363 - win32/x64
node: v16.14.0
yarn: 1.22.19
cli:  0.18.1-next.0 (local)
backstage:  N/A

Dependencies:
  @backstage/core-components         0.10.0
  @backstage/core-plugin-api         1.0.4
  @backstage/integration-react       1.1.2
  @backstage/integration             1.2.2
  @backstage/plugin-catalog-common   1.0.4
  @backstage/plugin-catalog-react    1.1.2
  @backstage/plugin-home             0.4.23
  @backstage/plugin-permission-react 0.4.3
  @backstage/plugin-stack-overflow   0.1.3
@tritri251214 tritri251214 added the bug Something isn't working label Aug 3, 2022
@github-actions github-actions bot added the area:scaffolder Everything and all things related to the scaffolder project area label Aug 3, 2022
@benjdlambert
Copy link
Member

I think this is because of how the state is set. It's just spread into the existing state, rather than replacing the existing state for each step. So if it's set once it's never reset. Do you want to have a go at fixing this bug? I think what we want to do is to keep an array for each step of the state then merge them together at the end rather than keeping a running state.

@tritri251214
Copy link
Contributor Author

@benjdlambert , I created PR for this issue.

PR: #12966

Could you help me review it?

Thanks.

@tritri251214
Copy link
Contributor Author

This issue fixed on PR #13017

@github-actions
Copy link
Contributor

github-actions bot commented Oct 9, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@eytanhanig
Copy link

This issue fixed on PR #13017

@tritri251214 I am still experiencing this issue when testing the original YAML you posted above with Backstage version 1.13.1. Can you confirm whether this is still working for you?

Here are the abbreviated results of running `yarn backstage-cli info:

yarn run v1.22.19
$ REDACTED/backstage/node_modules/.bin/backstage-cli info
OS:   Darwin 22.4.0 - darwin/arm64
node: v18.16.0
yarn: 1.22.19
cli:  0.22.6 (installed)
backstage:  1.13.1

Dependencies:
  ...
  @backstage/plugin-scaffolder-backend             1.13.1
  @backstage/plugin-scaffolder-common              1.2.7
  @backstage/plugin-scaffolder-node                0.1.2
  @backstage/plugin-scaffolder-react               1.3.0
  @backstage/plugin-scaffolder                     1.13.0
  ...

@benjdlambert
Copy link
Member

benjdlambert commented May 18, 2023

Can you try this with the scaffolder/next work that's been going on? We think that this is related to the underlying library which has been upgraded in the next version. https://backstage.io/docs/features/software-templates/testing-scaffolder-alpha/

@eytanhanig
Copy link

eytanhanig commented May 18, 2023

Hi @benjdlambert, thank you for the fast response!

I just tested using the master branch (6c0867b) which appears to come with scaffolder/next installed. So instead I just went to http://localhost:3000/settings/feature-flags and enabled scaffolder-next-preview there. I believe it worked since the display of step numbers switch from a vertical layout to a horizontal one. Let me know if that was the correct course of action.

Unfortunately even with the feature flag toggled on the same issue still occurred. I've attached a video below showing it.

Backstage_-_Scaffolder_Next_-_Issue_12948.mov

@benjdlambert benjdlambert reopened this May 19, 2023
@benjdlambert
Copy link
Member

Ah ok - I've re-opened this as I think that we need to add the omitExtraData prop to the form. Do you want to make this change and contribute? 🙏

@github-actions github-actions bot removed the stale label May 19, 2023
@eytanhanig
Copy link

@benjdlambert I'd be happy to give it a try, although I'll confess I'm not sure where to add it since I'm very new to TypeScript and this codebase.

@Rugvip Rugvip added the help wanted Help/Contributions wanted from community members label May 22, 2023
@benjdlambert
Copy link
Member

I think you might be able to add it to this:

<Form
validator={validator}
extraErrors={errors as unknown as ErrorSchema}
formData={formState}
formContext={{ formData: formState }}
schema={currentStep.schema}
uiSchema={currentStep.uiSchema}
onSubmit={handleNext}
fields={{ ...FieldOverrides, ...extensions }}
showErrorList={false}
onChange={handleChange}
{...(props.FormProps ?? {})}
and try it locally using the scaffolder/next route and see if that works 🤞

@eytanhanig
Copy link

Great minds think alike! Over the weekend I tried adding it to the following files:

  • plugins/scaffolder-react/src/next/components/Stepper/Stepper.tsx
  • plugins/scaffolder/src/next/TemplateEditorPage/CustomFieldExplorer.tsx

Unfortunately there was no change in behavior. I also tried adding liveOmit, but that similarly had no impact.

When you say "try it locally using the scaffolder/next route", does that encompass doing anything beyond using the master branch and enabling the scaffolder-next-preview feature flag in the settings UI?

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@TKA14EH
Copy link

TKA14EH commented Aug 17, 2023

Any update on a fix for the issue of Review and Create not resetting/hiding the form fields after changing selections?

Looking for a fix that doesn't include the experimental version of scaffolder/next plugin. omitExtraData added to typescript props doesn't seem to work.

@benjdlambert
Copy link
Member

benjdlambert commented Aug 21, 2023

Hmm so trying this in the editor in the new playground this seems to work upstream, so I think we just need to bump the library and turn on omitExtraData and possibly liveOmit. @TKA14EH we're moving the scaffolder/next work to be the default soon it's coming here: #19484

Here's something that seems to work here

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Oct 20, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 27, 2023
@Teemukoivumaa
Copy link
Contributor

@benjdlambert Should this work now? Tested this on our Backstage instance that uses the scaffolder/next but it doesn't omit the extra data

@benjdlambert
Copy link
Member

@Teemukoivumaa sorry I think this got stale. Will re-open this, do you want to perhaps raise a PR with the using the omitExtraData prop and see if it works for you?

@benjdlambert benjdlambert reopened this Jan 11, 2024
@github-actions github-actions bot removed the stale label Jan 11, 2024
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Mar 11, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 18, 2024
@Teemukoivumaa
Copy link
Contributor

@benjdlambert Got some time (and motivation) to try to fix this, but it just doesn't seem to work. I have set the omitExtraData & liveOmit to true. But it doesn't remove the hidden fields like it does in the RJSF playground.

See the demo video:

2024-03-24.14-21-15.mp4

@Teemukoivumaa
Copy link
Contributor

Still a problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:scaffolder Everything and all things related to the scaffolder project area bug Something isn't working help wanted Help/Contributions wanted from community members
Projects
None yet
6 participants