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

feat(NOJIRA-123): update form.settings type to have all options #119

Merged
merged 3 commits into from
Feb 22, 2024
Merged

feat(NOJIRA-123): update form.settings type to have all options #119

merged 3 commits into from
Feb 22, 2024

Conversation

gurpreetatwal
Copy link
Contributor

I got the list of defaults by creating a new form using the API and inspecting the response as some of the options were not listed on the docs / had different names vs what was specified on the docs.

I also reordered the options to match the API response order.

Added:

  • are_uploads_public
  • autosave_progress (docs have this as autocomplete_progress)
  • free_form_navigation
  • hide_navigation
  • pro_subdomain_enabled
  • show_cookie_consent
  • show_key_hint_on_choices
  • show_number_of_submissions
  • show_question_number
  • show_time_to_complete
  • use_lead_qualification

Reordered:

  • language
  • is_public
  • progress_bar
  • show_progress_bar
  • show_typeform_branding

I got the list of defaults by creating a new form using the API and
inspecting the response as some of the options were not listed on the
docs / had different names vs what was specified on the docs.

I also reordered the options to match the API response order.

Added:
- are_uploads_public
- autosave_progress (docs have this as `autocomplete_progress`)
- free_form_navigation
- hide_navigation
- pro_subdomain_enabled
- show_cookie_consent
- show_key_hint_on_choices
- show_number_of_submissions
- show_question_number
- show_time_to_complete
- use_lead_qualification

Reordered:
- language
- is_public
- progress_bar
- show_progress_bar
- show_typeform_branding
@gurpreetatwal gurpreetatwal requested a review from a team as a code owner February 16, 2024 01:11
@mathio
Copy link
Contributor

mathio commented Feb 19, 2024

Hello @gurpreetatwal thank you for the PR. It looks good, I have just one question - where did you get the "default" values from? If I am not mistaken, if the field is not present then the default value should be false.

@mathio mathio changed the title feat(type): update form.settings type to have all options feat(NOJIRA-123): update form.settings type to have all options Feb 19, 2024
@gurpreetatwal
Copy link
Contributor Author

Hi @mathio, thank you for the quick reply! I pulled the default value from the API response after creating a new form (e.g. create a form using the API with no options set and then inspect the response from the corresponding GET request). In that sense the defaults I documented might "go too far" in that they're listing the default behavior of Typeform and not the interface specifically.

Happy to change the defaults to undefined if the aim is to document the default of the interface itself

@mathio
Copy link
Contributor

mathio commented Feb 20, 2024

Thanks. I talked to my team mates and we think an undefined as a default is fine, as this is "going too far" 👍

@gurpreetatwal
Copy link
Contributor Author

Sounds good! In terms of documentation which of the following should I do?

  1. Remove the default mention entirely
  2. Update default to be undefined
  3. Update default to be "" (matches convention in rest of file)

@mathio
Copy link
Contributor

mathio commented Feb 21, 2024

I think you can match the current convention.

@mathio mathio merged commit 16ed6f3 into Typeform:main Feb 22, 2024
5 checks passed
@typeform-ops-gha
Copy link

🎉 This PR is included in version 2.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

None yet

3 participants