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

Set textarea value only when set. #91

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MattHolmes123
Copy link

@MattHolmes123 MattHolmes123 commented Dec 2, 2024

Example below showing problem with govukTextarea rendering None when default is None

  <h3>Example govukInput with default value of None</h3>
  {{ govukInput({
      "label": {
        "text": "What is the name of the event?",
        "classes": "govuk-label--l",
        "isPageHeading": true
      },
      "id": "event-name",
      "name": "eventName",
      "value": None
    }) }}

  <h3>Example govukTextarea with default value of None</h3>
  {{ govukTextarea({
    "name": "moreDetail",
    "id": "more-detail",
    "label": {
      "text": "Can you provide more detail?",
      "classes": "govuk-label--l",
      "isPageHeading": true
    },
    "hint": {
      "text": "Do not include personal or financial information, like your National Insurance number or credit card details"
    },
    "value": None
  }) }}

image

@MattHolmes123
Copy link
Author

Would you like me to update the vulnerability in this PR?
image

@matthew-shaw
Copy link
Member

Hi @MattHolmes123 thanks for the contribution 🎉

Could you just omit passing the value key to the macro? It seems that for the textarea component this is coerced into a string, but not for the input component. This is consistent with the original GOV.UK Frontend component macro behaviour too, so while it could handle it more gracefully, it would deviate from the source material.

If you're creating and validating form field inputs, have you considered using GOV.UK Frontend WTForms?

@MattHolmes123
Copy link
Author

Hi @matthew-shaw.

Thanks for the recommendation but I'm working with Django.
I will close this PR then as I understand the need to not deviate away from the source material.
I may even raise it as an issue in the original GOV.UK frontend project.

Great project btw it's helped my team loads. 👍

@MattHolmes123
Copy link
Author

Hi @matthew-shaw
So I checked out the govuk-frontend project and tested it and the bug isn't found in their version.

I manually changed the template.njk file to this and it didn't render "null" in the text area:
{{- govukAttributes(params.attributes) }}>{{ null }}</textarea>

If you look at the Nunjucks docs for variables it does say "If a value is undefined or null, nothing is displayed".

So will leave it with you if you want this change or not.
Thanks 👍

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.

2 participants