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

(refactor) FormField attribute 'Type' should be Optional #419

Closed

Conversation

Twiineenock
Copy link
Contributor

@Twiineenock Twiineenock commented Oct 25, 2024

Requirements

  • This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
  • My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
  • My work includes tests or is validated by existing tests.

Summary

Markdown questions have no question type hence should be optional in the form schema.
This related to the work coming in from here We should make this optional since markdown content doesn't have a type

Screenshots

Related Issue

Other

@NethmiRodrigo NethmiRodrigo changed the title feat: FormField Type should be Optioanl (refactor) FormField attribute 'Type' should be Optioanl Oct 25, 2024
Copy link
Contributor

@NethmiRodrigo NethmiRodrigo left a comment

Choose a reason for hiding this comment

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

LGTM! @samuelmale we had to make type optional as well. Does that seem okay?

@NethmiRodrigo NethmiRodrigo changed the title (refactor) FormField attribute 'Type' should be Optioanl (refactor) FormField attribute 'Type' should be Optional Oct 27, 2024
@samuelmale
Copy link
Member

This makes sense and I'm not opposed to the idea.

Markdown questions have no question type hence should be optional in the form schema

The FE defines a pseudo type called control (see) and I believe it should be used in such scenarios. If this makes sense, we can have this type as a reasonable default in the form-builder. cc: @denniskigen

           {
             "type": "control"
              "value": [
                "**This form is used to**:showcase feature of the react form engine",
                "### This is just an **explainer text** example  in *markdown*"
              ],
              "questionOptions": {
                "rendering": "markdown"
              },
              "id": "heading2"
            }

@denniskigen
Copy link
Member

Seems reasonable to me, @samuelmale!

@NethmiRodrigo
Copy link
Contributor

@samuelmale that makes sense! Shall we opt to do that instead then, default type to the value control, without making it optional? FYI @Twiineenock

@samuelmale
Copy link
Member

Shall we opt to do that instead then, default type to the value control, without making it optional?

Makes sense to me!

@NethmiRodrigo
Copy link
Contributor

Closing this PR in favor of making the type to control for markdown content

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants