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

REGRESSION: dropdown changes requires the selectedItem to store entire option array item (label and value) to use template selecteditem #14172

Closed
pete-mcwilliams opened this issue Nov 21, 2023 · 20 comments
Assignees
Labels
Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Milestone

Comments

@pete-mcwilliams
Copy link
Contributor

pete-mcwilliams commented Nov 21, 2023

Describe the bug

Previously the selected template would pass the entire option array item to the pTemplate="selectedItem", both the optionLabel and optionValue could be passed as parameters into p-dropdown, this allowed the optionValue to be stored in the ngModel.

The change has caused selectedItem template to receive only the value when optionValue is specified. The entire option item has to be stored as the selected value so that the label can be available in the template. This can be seen on the https://primeng.org/dropdown#template

It is undesirable to have to store the code and label as the selected item, imagine trying to change the label in the future in a json structure in a database for all rows.

16.8.0 demo use of dropdown with templates (breaking change requiring selectedItem to be entire option array item)
https://stackblitz.com/edit/zkmdqg?file=src%2Fapp%2Fdemo%2Fdropdown-template-demo.html

16.8.0 this should work allowing only the code to be persisted but does not as the template is only passed the optionValue.
https://stackblitz.com/edit/zkmdqg-buomly?file=src%2Fapp%2Fdemo%2Fdropdown-template-demo.html,package.json

16.4.2 working as it should same example as above with only primeng version change
https://stackblitz.com/edit/zkmdqg-ivcsef?file=src%2Fapp%2Fdemo%2Fdropdown-template-demo.html,package.json

Environment

see repros

Reproducer

https://stackblitz.com/edit/zkmdqg-buomly?file=src%2Fapp%2Fdemo%2Fdropdown-template-demo.html,package.json

Angular version

16.2.7

PrimeNG version

16.8.0

Build / Runtime

Angular CLI App

Language

TypeScript

Node version (for AoT issues node --version)

v18.17.0

Browser(s)

n/a

Steps to reproduce the behavior

use the repros

Expected behavior

we should be able to store a property of the option item and be able to use the pTemplate="selectedItem"

@cetincakiroglu
Copy link
Contributor

Also related to #14171

@cetincakiroglu
Copy link
Contributor

Thanks a lot for reporting the issue!

@cetincakiroglu cetincakiroglu added the Type: Bug Issue contains a bug related to a specific component. Something about the component is not working label Nov 22, 2023
@cetincakiroglu cetincakiroglu self-assigned this Nov 22, 2023
@cetincakiroglu cetincakiroglu removed the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Nov 22, 2023
@jerkovicl
Copy link

jerkovicl commented Nov 22, 2023

@cetincakiroglu so selectedOption prop is back, after it was removed right? 😁

@jerkovicl
Copy link

jerkovicl commented Nov 23, 2023

still not fixed, tested with 16.9.0 @cetincakiroglu
still getting constant errors about item undefined in selectedItem template

image

@cetincakiroglu
Copy link
Contributor

still not fixed, tested with 16.9.0 @cetincakiroglu

still getting constant errors about item undefined in selectedItem template

image

Could you please share an example?

@pete-mcwilliams
Copy link
Contributor Author

The issue I had on p-dropdown has been resolved on 16.9.0 https://stackblitz.com/edit/zkmdqg-gyeyzs?file=src%2Fapp%2Fdemo%2Fdropdown-template-demo.html,package.json

Looks like a different issue.

@jerkovicl
Copy link

@pete-mcwilliams yeah that demo works, but i am getting errors in project, i have to isolate this reproduction case, will try to provide example

@Phong6698
Copy link

The issue is resolved for me as well.

I had check if selectedItem is undefined since it also threw an error for me at first.

<ng-template let-selectedItem pTemplate="selectedItem">
  <div *ngIf="selectedItem" class="ml-2">
    <i [class]="selectedItem.icon" class="mr-3"></i>{{selectedItem.label}}
  </div>
</ng-template>

@jerkovicl
Copy link

The issue is resolved for me as well.

I had check if selectedItem is undefined since it also threw an error for me at first.

<ng-template let-selectedItem pTemplate="selectedItem">
  <div *ngIf="selectedItem" class="ml-2">
    <i [class]="selectedItem.icon" class="mr-3"></i>{{selectedItem.label}}
  </div>
</ng-template>

if i do that then content inside template is not shown even tough value is there

@eXpertise7
Copy link

eXpertise7 commented Nov 27, 2023

@jerkovicl @pete-mcwilliams Sorry to bother, but do any of you guys have [filter]="true" at any of your p-dropdown components?

I'm almost sure that filter of p-dropdown does not work as expected. To have filter working, we need to implicity specifiy optionValue. If we specifiy optionValue implicitly , [(ngModel)] variable value (with p-dropdown with pTemplate's selectedItem and item) is no longer object, it's a string.

Need to investigate more. Will update.

@jerkovicl
Copy link

@eXpertise7 yes i have filter and autoDisplayFirst both and separately on some of dropdowns,
there is still some case where value with pTemplate is string and not a object like you mentioned, but didnt find the non working combination yet

@pete-mcwilliams
Copy link
Contributor Author

ngModel should be the property specified in optionValue, that gives you link by reference to the option array rather than having to store the entire option as key to the options array (including the label).

Can you provide a stackblitz repro showing your issue?

@jerkovicl
Copy link

jerkovicl commented Nov 27, 2023

@eXpertise7 another thing i noticed is if you use any pipe like async or translate inside pTemplate then label errors show up aswell

like this:

  <div class="field">
            <p-dropdown
              required
              formControlName="criterionType"
              [placeholder]="sharedTranslateKeys.FilterSelectCondition | translate"
              [id]="'criterionType' + i"
              [name]="'criterionType' + i"
              [options]="propertySelect.selectedOption?.conditions"
              [optionLabel]="'label'"
              [optionValue]="'value'"
              [attr.data-cy]="'filter-control-type-' + i"
            >
              <ng-template let-criterionTypeSelectedProp pTemplate="selectedItem">
                <span>{{ $any(SharedTranslateKeys)[criterionTypeSelectedProp.label] | translate }}</span>
              </ng-template>
              <ng-template let-criterionTypeProp pTemplate="item">
                <span>{{ $any(SharedTranslateKeys)[criterionTypeProp.label] | translate }}</span>
              </ng-template>
            </p-dropdown>
  </div>

@pete-mcwilliams
Copy link
Contributor Author

I found I had to change to something like this from your example
{{ criterionTypeSelectedProp?.label ?? '' | translate }}

@jerkovicl
Copy link

jerkovicl commented Nov 27, 2023

@pete-mcwilliams as soon as you do ? on label prop then dropdown with selected value will look like this, error will be gone tough:

image

but if i remove null check then dropdown shows selected label but throws error

image

translate pipe is a non pure pipe tough so it triggers on every change detection

@eXpertise7
Copy link

eXpertise7 commented Nov 27, 2023

as soon as you do ? on label prop then dropdown with selected value will look like this, error will be gone tough:

True, I can confirm it. I believe that happens because p-dropdown makes new DOM span element in scenario you have.

image

In the image above: Where "MY ->" is - that's my pTemplate="selectedItem" variable, but in the above that's the new empty DOM span element which breaks my p-dropdown visually, it could be breaking yours also.

You can manipulate it's styles in component CSS file though :host ::ng-deep selector.

In my example I had to do this to make stuff visually normally initially:

.p-dropdown-label.p-inputtext {
    display: flex;
    width: 34px;
}

We should be asking @cetincakiroglu why p-dropdown works differently now, or we could be putting some hours into p-dropdown codebase and understanding what are the changes now.

@jerkovicl
Copy link

@eXpertise7 good find, i am sure the cause is refactoring because on v16.6.0 everything worked normally

@eXpertise7
Copy link

eXpertise7 commented Nov 27, 2023

Just a small update - about the issue above I stated with [filter]="true", well, I think I've made stuff fully work at my p-dropdown (every feature works).

I needed to implicity put property I want to filterBy to have p-dropdown filter working. Before I did not have to do that.

This is what will make people report issues or get them confused. I'm working with this awesome UI library for 6 years, and these breaking changes will frustrate people - stuff will not work on [email protected], if data bound to p-dropdown is not checked.

@jerkovicl
Copy link

@eXpertise7 tried workaround with css but didnt help in my case

@jerkovicl
Copy link

related to #14241

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Projects
None yet
Development

No branches or pull requests

5 participants