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

bug: ion-select popover does show the selected value in the opened popover #25759

Closed
4 of 7 tasks
ryaa opened this issue Aug 15, 2022 · 4 comments · Fixed by #25764
Closed
4 of 7 tasks

bug: ion-select popover does show the selected value in the opened popover #25759

ryaa opened this issue Aug 15, 2022 · 4 comments · Fixed by #25764
Assignees
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@ryaa
Copy link

ryaa commented Aug 15, 2022

Prerequisites

Ionic Framework Version

  • v4.x
  • v5.x
  • v6.x
  • Nightly

Current Behavior

The ion-select with multiple=true and compareWith function does not show the selected value in the opened popover when the selected value is string and the value to compare against is number. compareWith works correctly returning true for the selected value.
NOTE: if both, the selected value and the value to compare against are numbers, all work fine.

Expected Behavior

The opened popover must show the selected value

Steps to Reproduce

  1. open this link https://ionic6-angular13-lfygtv.stackblitz.io
    Note the selected value (text) is shown correctly
  2. tap on the select control
    Note that in the opened popover no value is selected - see below

Screen Shot 2022-08-15 at 09 07 44

Code Reproduction URL

https://stackblitz.com/edit/ionic6-angular13-lfygtv?file=src/app/app.component.html

Ionic Info

See https://stackblitz.com/edit/ionic6-angular13-lfygtv?file=src/app/app.component.html

Additional Information

if both, the selected value and the value to compare against are numbers, all work fine.

@ionitron-bot ionitron-bot bot added the triage label Aug 15, 2022
@liamdebeasi liamdebeasi self-assigned this Aug 15, 2022
@liamdebeasi
Copy link
Contributor

liamdebeasi commented Aug 15, 2022

Thanks for the issue. The problem here is we reverse the order of the parameters when checking if the checkboxes are checked or not. As a result, val1 is the string value and val2 is the number value. You can fix this by converting both val1 and val2 to a number.


However, this is something we should fix. When we rendering the selected text in the ion-select label, we pass in the option value first and the ion-select value second:

return compareOptions(getOptionValue(opt), value, compareWith);

When creating the alert checkboxes we pass in the ion-select value first and the option value second:

checked: isOptionSelected(selectValue, value, this.compareWith),
(Note: even though the second variable is called value, it is actually the value of the option, not the value of the select component.

However, the underlying implementation for isOptionSelected expects the ion-select value first and the compare value second.

@liamdebeasi liamdebeasi added package: core @ionic/core package type: bug a confirmed bug report labels Aug 15, 2022
@ionitron-bot ionitron-bot bot removed the triage label Aug 15, 2022
@liamdebeasi
Copy link
Contributor

liamdebeasi commented Aug 16, 2022

Here is a dev build if you are interested in giving the fix a test:

npm install @ionic/[email protected]

Note: You may need to test this in a local Ionic application. StackBlitz has some issues with installing dev builds in non-Web Container instances.

edit: One thing to note is that the first parameter will be the ion-select value and the second parameter will be the value of the option, so you may need to switch which parameter gets converted to a number.

@liamdebeasi
Copy link
Contributor

Thanks for the issue. This has been resolved via #25764, and a fix will be available in an upcoming release of Ionic Framework. Please feel free to continue testing the dev build.

Note: As a result of this change, the param order will now match what is stated on the docs: https://ionicframework.com/docs/api/select#comparewith

The first parameter will be the ion-select value, and the second parameter will be the value of an ion-select-option.

@ionitron-bot
Copy link

ionitron-bot bot commented Sep 21, 2022

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Sep 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants