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

fix: display ontology labels in prefffered language (DEV-3739) #1651

Merged

Conversation

mpro7
Copy link
Collaborator

@mpro7 mpro7 commented Jun 14, 2024

resolves DEV-3739

@mpro7 mpro7 self-assigned this Jun 14, 2024
Copy link

linear bot commented Jun 14, 2024

@mpro7 mpro7 marked this pull request as ready for review June 18, 2024 08:42
@mpro7 mpro7 requested a review from derschnee68 June 18, 2024 09:04
matTooltipShowDelay="750"
[matTooltipPosition]="'above'"
[matTooltipDisabled]="tooltipDisabled">
{{ resClass.label }}
{{ displayOntologyLabelsInPreferredLanguage() }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would not use a method in the view, as it calls it many many times during the rerender

@@ -36,7 +37,7 @@ import { map, takeUntil } from 'rxjs/operators';
export class OntologyClassItemComponent implements OnInit, AfterViewInit, OnDestroy {
destroyed: Subject<void> = new Subject<void>();

@Input() resClass: ClassDefinition;
@Input() resClass: ResourceClassDefinitionWithAllLanguages;
Copy link
Collaborator

Choose a reason for hiding this comment

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

add "?" if it is not required, otherwise @input({required: true}) variable!;

prefferedLanguage = l;
});

if (this.resClass.labels) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should go in the subscribe

Copy link
Collaborator

Choose a reason for hiding this comment

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

const preferedLanguage = this._store.selectSnapshot(UserSelector.language);

@@ -66,6 +67,8 @@ export class OntologyClassItemComponent implements OnInit, AfterViewInit, OnDest
},
};

ontologiesLabel: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

ontologiesLabel?: string;

// https://github.com/dasch-swiss/dsp-das/pull/1651
const prefferedLanguage = this._store.selectSnapshot(UserSelectors.language);

if (this.resClass.labels) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

so if this condition is not met, ontologiesLabel stays undefined, is that what we want ? (just asking I don't know what's the right behaviour)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this is not necessary, because of the guard I added: this.resClass.labels[0].value. If there is no match between label languages and preferred language, the first one from the labels array is applied, and at least one label is required AFAIK.

@mpro7 mpro7 merged commit 2dfd44d into main Jun 18, 2024
13 checks passed
@mpro7 mpro7 deleted the dev-3739-labels-arent-displayed-in-the-users-preferred-language branch June 18, 2024 11:26
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.

2 participants