Skip to content

fix/AB#106122_ABC-resource-question-display-glitches #2713

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

Open
wants to merge 5 commits into
base: beta
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -80,17 +80,7 @@ export class ResourceSelectComponent extends GraphQLSelectComponent {
this.valueField = 'id';
this.textField = 'name';
this.filterable = true;
this.searchChange.pipe(takeUntil(this.destroy$)).subscribe((value) => {
this.onSearchChange(value);
});
}

/**
* Override GraphQLSelectComponent onOpenSelect to only load query when
* select menu is open for the first time.
*
*/
public override onOpenSelect(): void {
/** Initialize resource query with the component automatically*/
if (!this.query) {
this.query = this.apollo.watchQuery<ResourcesQueryResponse>({
query: GET_RESOURCES,
Expand All @@ -99,15 +89,10 @@ export class ResourceSelectComponent extends GraphQLSelectComponent {
sortField: 'name',
},
});

this.query.valueChanges
.pipe(takeUntil(this.queryChange$), takeUntil(this.destroy$))
.subscribe(({ data, loading }) => {
this.queryName = Object.keys(data)[0];
this.updateValues(data, loading);
});
}
super.onOpenSelect();
this.searchChange.pipe(takeUntil(this.destroy$)).subscribe((value) => {
this.onSearchChange(value);
});
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<shared-resource-select
[isSurveyQuestion]="true"
[formControl]="resourceControl"
[selectedElements]="[selectedResource]"
[selectedElements]="selectedResource"
>
</shared-resource-select>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ import {
} from '@angular/core';
import { UntypedFormControl } from '@angular/forms';
import { Apollo } from 'apollo-angular';
import { Subject } from 'rxjs';
import { takeUntil } from 'rxjs/operators';
import { QuestionAngular } from 'survey-angular-ui';
import {
Resource,
ResourceQueryResponse,
} from '../../../models/resource.model';
import { GET_SHORT_RESOURCE_BY_ID } from './graphql/queries';
import { takeUntil } from 'rxjs/operators';
import { QuestionAngular } from 'survey-angular-ui';
import { QuestionResourceDropdownModel } from './resource-dropdown.model';
import { Subject } from 'rxjs';

/**
* This component is used to create a dropdown where the user can select a resource.
Expand All @@ -29,7 +29,7 @@ export class ResourceDropdownComponent
implements OnInit
{
/** Selected resource */
public selectedResource?: Resource;
public selectedResource: Resource[] = [];
/** Resource control */
public resourceControl!: UntypedFormControl;

Expand All @@ -54,13 +54,6 @@ export class ResourceDropdownComponent

override ngOnInit(): void {
super.ngOnInit();
this.resourceControl = new UntypedFormControl(this.model.value ?? '');
this.resourceControl.valueChanges
.pipe(takeUntil(this.destroy$))
.subscribe((value) => {
this.model.value = value;
this.model.obj.gridFieldsSettings = null;
});
if (this.model.value) {
this.apollo
.query<ResourceQueryResponse>({
Expand All @@ -71,9 +64,17 @@ export class ResourceDropdownComponent
})
.subscribe(({ data }) => {
if (data.resource) {
this.selectedResource = data.resource;
this.selectedResource = [data.resource];
this.changeDetectorRef.detectChanges();
}
});
}
this.resourceControl = new UntypedFormControl(this.model.value ?? '');
this.resourceControl.valueChanges
.pipe(takeUntil(this.destroy$))
.subscribe((value) => {
this.model.value = value;
this.model.obj.gridFieldsSettings = null;
});
}
}
7 changes: 4 additions & 3 deletions libs/ui/src/lib/graphql-select/graphql-select.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -533,9 +533,10 @@ export class GraphQLSelectComponent
const selectedElements = this.selectedElements.filter(
(selectedElement) =>
selectedElement &&
!elements.find(
(node) => node[this.valueField] === selectedElement[this.valueField]
)
(this.isSurveyQuestion ||
!elements.find(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@unai-reliefapp
to be more generic, perhaps we should find another way to do it
I believe the issue is due to the fact that the selectedItem is not taken into consideration when you provide a new list of choices, and it's not only affecting the resource-dropdown component, but I got the feeling we've already managed to fix that in some of the instances of the graphql select component

from what I know, it's quicker to fetch the single selected resource than the first 10, and so it creates this error

I'll put back the ticket in todo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem was that the change detector ref was not working as expected due to the integration in the survey js and that we are using and extended component from graphql component. In the rest of the graphql instances in the application the default logic is working fine because of the change detector ref is working as expected. This way i make sure that on surveyjs the default logic to trigger the selected item outside the default fetche list is also added.

What do you have in mind for this then?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@unai-reliefapp
I can see a lot of change detector call in the stack, they don't work ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No to this component level deepness, with surveyjs and an extended graphql component. That's why I used the isSurveyQuestion flag on this one to trigger the selected element insertion outside the default fetched list

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, I'll have a look at it then
thanks for the explanation 👍

(node) => node[this.valueField] === selectedElement[this.valueField]
))
);
this.cachedElements = updateQueryUniqueValues(this.cachedElements, [
...selectedElements,
Expand Down