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

Make prefix optional, at the moment it always renders prefix #165

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

Conversation

shivanshuit914
Copy link
Contributor

@shivanshuit914 shivanshuit914 commented Mar 5, 2024

@BecioProton @georgeeker

Make prefix optional, at the moment it always renders prefix.

@shivanshuit914 shivanshuit914 force-pushed the feature/prefix-optional branch from 9f8596d to 3e4d0f1 Compare March 5, 2024 13:56
@shivanshuit914 shivanshuit914 changed the title Feature/prefix optional Make prefix optional, at the moment it always renders prefix Mar 5, 2024
@shivanshuit914 shivanshuit914 force-pushed the feature/prefix-optional branch 2 times, most recently from b9f6393 to dbb5694 Compare March 5, 2024 14:19
@HughePaul
Copy link
Contributor

I'd you specify null or undefined it shouldn't get rendered. If you specify a string, even if empty, it will still render that empty string as that is what you have asked the component to do

@shivanshuit914 shivanshuit914 force-pushed the feature/prefix-optional branch from dbb5694 to 1772a0b Compare March 10, 2024 20:09
@shivanshuit914
Copy link
Contributor Author

shivanshuit914 commented Mar 10, 2024

I'd you specify null or undefined it shouldn't get rendered. If you specify a string, even if empty, it will still render that empty string as that is what you have asked the component to do

Thanks for the feedback. I have added test for undefined and empty string.

I have tested it in my local env.

{{ hmpoText(ctx, {
      id: "question",
      name: "question",
      label: {
        text: question.label,
        classes: "govuk-label--m"
      },
      hint: {
        text: question.hint
      },
      classes: "govuk-input--width-5"
    }) }}
    
Screenshot 2024-03-10 at 20 10 19

@AmritSidhu
Copy link

@HughePaul are you able to get this work merged in?

AmritSidhu added a commit to govuk-one-login/ipv-cri-kbv-hmrc-front that referenced this pull request Mar 22, 2024
We are temporarily downgrading hmpo-components

We made a change in hmpo-components to add the prefix feild to the text inputs, however this change made it a required feild and therefore cant use the input field without a prefix
There is a current PR HMPO/hmpo-components#165 to fix this issue which is awaiting merge from HMPO

It was decided that for now and to unblock our tickets that we revert to an earlier version of hmpo-components and just use the plain text input field until the above PR gets merged.
blakeyp added a commit to govuk-one-login/ipv-cri-kbv-hmrc-front that referenced this pull request May 20, 2024
We’re still waiting for hmpo-components PR HMPO/hmpo-components#165  to be merged and released. In the meantime we should implement a workaround so that amount fields have the “£” prefix while non-amount fields don’t.
blakeyp added a commit to govuk-one-login/ipv-cri-kbv-hmrc-front that referenced this pull request May 21, 2024
We’re still waiting for hmpo-components PR HMPO/hmpo-components#165  to be merged and released. In the meantime we should implement a workaround so that amount fields have the “£” prefix while non-amount fields don’t.
blakeyp added a commit to govuk-one-login/ipv-cri-kbv-hmrc-front that referenced this pull request May 21, 2024
We’re still waiting for hmpo-components PR HMPO/hmpo-components#165  to be merged and released. In the meantime we should implement a workaround so that amount fields have the “£” prefix while non-amount fields don’t.
blakeyp added a commit to govuk-one-login/ipv-cri-kbv-hmrc-front that referenced this pull request May 22, 2024
We’re still waiting for hmpo-components PR HMPO/hmpo-components#165  to be merged and released. In the meantime we should implement a workaround so that amount fields have the “£” prefix while non-amount fields don’t.
@SamChatfield SamChatfield force-pushed the master branch 3 times, most recently from 307e703 to 6403fcc Compare August 1, 2024 16:28
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