alwaysRestoreSelectedOptionsMulti option not respected

[jamiemac87]

I have an issue in my app that I think would be resolved if the option alwaysRestoreSelectedOptionsMulti="false" was being respected. However it looks as though we also add the following condition (this._formControl.value && this._formControl.value.length) when determining if selected options should be restored. See here.

Is there a reason for the additional condition?

I would like to test out potential fixes locally but I have never really contributed to any open source work before. Would someone be able to direct me to any resources that might help me understand how I would go about this. My basic understanding is I would fork the repo to make my changes and create a pull request from there. However how would I go about building the library and pointing my local app to use my version of the library?

[macjohnny]

the description of alwaysRestoreSelectedOptionsMulti states

ngx-mat-select-search/src/app/mat-select-search/mat-select-search.component.ts

Lines 196 to 200 in a75373e

	/**
	* Always restore selected options on selectionChange for mode multi (e.g. for lazy loading/infinity scrolling).
	* Defaults to false, so selected options are only restored while filtering is active.
	*/
	@Input() alwaysRestoreSelectedOptionsMulti = false;

options are only restored while filtering is active

this refers to the this._formControl.value && this._formControl.value.length part in

ngx-mat-select-search/src/app/mat-select-search/mat-select-search.component.ts

Line 586 in a75373e

if ((this.alwaysRestoreSelectedOptionsMulti || (this._formControl.value && this._formControl.value.length))

setting alwaysRestoreSelectedOptionsMulti=true enhances the restoration behavior to also work when no search term was entered, to fix infinity scrolling behavior, see #320 and #324

My basic understanding is I would fork the repo to make my changes and create a pull request from there.

That's correct, and potentially explain the problem you are trying to solve in the issue.

However how would I go about building the library and pointing my local app to use my version of the library?

Since there is a demo app inside this project, running

git clone [email protected]:bithost-gmbh/ngx-mat-select-search.git
cd ngx-mat-select-search
npm install
ng serve

will start up a demo app that can be accessed in the browser via the url http://localhost:4200.

You can then modify the code in the examples src/app/examples as well as the library itself src/app/mat-select-search to test your changes.

If you want to build the library, run

npm run build-lib

which will create a folder dist-lib that contains the locally built npm-package.
You can then rename dist-lib to ngx-mat-select-search and copy that to the node_modules folder of your app.

see also https://github.com/bithost-gmbh/ngx-mat-select-search#development

[jamiemac87]

Thank you for the detailed reply.

So I have some behaviour where the user filters the options and upon hitting enter key we would like the filtered options to be selected and any previous selections to be unselected. Is this possible with the latest version of the library? This behaviour worked in version 3.1.2 before the alwaysRestoreSelectedOptionsMulti option was added/fixed. If not I will look to fork the library but just thought I would check first before undertaking that work

[macjohnny]

@jamiemac87 by not setting [alwaysRestoreSelectedOptionsMulti], i.e. using the default alwaysRestoreSelectedOptionsMulti = false, you get the same behavior as in 3.2.2, i.e. the version before alwaysRestoreSelectedOptionsMulti was added, see https://github.com/bithost-gmbh/ngx-mat-select-search/blob/master/CHANGELOG.md#330

However, after 3.1.2, there was an additional bugfix regarding the [multi]="true" behavior, see https://github.com/bithost-gmbh/ngx-mat-select-search/blob/master/CHANGELOG.md#313

if you want, we can introduce an option to achieve the behavior you intend to implement. Feel free to send a PR.

[macjohnny]

@jamiemac87 any update here?

[jamiemac87]

@macjohnny sorry about the late reply. Work has been busy so not had much time to dedicate to my fix. I have a draft MR up if you wanted to have a quick look. It accomplishes what I set out in #356 (comment) but does not introduce a new option so may not be the desired implementation and I have not done any thorough testing other than run the app locally to check for regressions

[macjohnny]

closing due to inactivity. feel free to update and i will reopen