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

DSP-652 FIX: Boolean add button #182

Merged
merged 7 commits into from Sep 21, 2020
Expand Up @@ -36,10 +36,7 @@
</div>
<!-- Add button -->
<!-- temporary hack to hide add button for an XML value -->
<div *ngIf="prop.guiDef.propertyIndex !== 'http://0.0.0.0:3333/ontology/0001/anything/v2#hasRichtext' &&
addButtonIsVisible &&
prop.guiDef.cardinality !== 1 &&
propID !== prop.propDef.id">
<div *ngIf="addValueIsAllowed(prop) && prop.guiDef.propertyIndex !== 'http://0.0.0.0:3333/ontology/0001/anything/v2#hasRichtext'">
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does the condition contain a property IRI from the anything test project?

Copy link
Contributor

Choose a reason for hiding this comment

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

ah I get it: but then this should be more generic than 'http://0.0.0.0:3333/ontology/0001/anything/v2#hasRichtext', you could use the gui ele

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should integrate the XML / CKeditor functionality asap, so this becomes unnecessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, I'll make the small change to use the gui element

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in 94ab6a8

<button class="create" (click)="showAddValueForm(prop)" title="Add a new value">
<mat-icon>add_box</mat-icon>
</button>
Expand Down
Expand Up @@ -217,12 +217,34 @@ describe('PropertyViewComponent', () => {
testHostFixture.detectChanges();
});

it('should show an add button under each property that has a value component and for which the cardinality is not 1', () => {
it('should show an add button under each property that has a value component and value and appropriate cardinality', () => {
const addButtons = propertyViewComponentDe.queryAll(By.css('button.create'));
expect(addButtons.length).toEqual(13);

});

it('should show an add button under a property with a cardinality of 1 and does not have a value', () => {

// show all properties so that we can access properties with no values
testHostComponent.showAllProps = true;
testHostFixture.detectChanges();

let addButtons = propertyViewComponentDe.queryAll(By.css('button.create'));

// current amount of buttons should equal 18 because the boolean property shouldn't have an add button if it has a value
expect(addButtons.length).toEqual(18);

// remove value from the boolean property
testHostComponent.propArray[9].values = [];

testHostFixture.detectChanges();

// now the boolean property should have an add button so the amount of add buttons on the page should increase by 1
addButtons = propertyViewComponentDe.queryAll(By.css('button.create'));
expect(addButtons.length).toEqual(19);

});

it('should show an add value component when the add button is clicked', () => {
const addButtonDebugElement = propertyViewComponentDe.query(By.css('button.create'));
const addButtonNativeElement = addButtonDebugElement.nativeElement;
Expand Down
@@ -1,12 +1,11 @@
import { animate, state, style, transition, trigger } from '@angular/animations';
import {
Component,
Input,
OnDestroy,
OnInit,
ViewChild
} from '@angular/core';
import { PermissionUtil, ReadResource, SystemPropertyDefinition } from '@dasch-swiss/dsp-js';
import { CardinalityUtil, PermissionUtil, ReadResource, SystemPropertyDefinition } from '@dasch-swiss/dsp-js';
import { Subscription } from 'rxjs';
import { AddValueComponent } from '../../operations/add-value/add-value.component';
import { DisplayEditComponent } from '../../operations/display-edit/display-edit.component';
Expand Down Expand Up @@ -53,7 +52,6 @@ export class PropertyViewComponent implements OnInit, OnDestroy {
addButtonIsVisible: boolean; // used to toggle add value button
addValueFormIsVisible: boolean; // used to toggle add value form field
propID: string; // used in template to show only the add value form of the corresponding value
readOnlyProp: boolean; // used in template to not show an "add" button for properties we do not yet have a way to create/edit

valueOperationEventSubscription: Subscription;

Expand Down Expand Up @@ -86,10 +84,8 @@ export class PropertyViewComponent implements OnInit, OnDestroy {
* Called from the template when the user clicks on the add button
*/
showAddValueForm(prop: PropertyInfoValues) {

this.propID = prop.propDef.id;
this.addValueFormIsVisible = true;

}

/**
Expand All @@ -100,4 +96,24 @@ export class PropertyViewComponent implements OnInit, OnDestroy {
this.addButtonIsVisible = true;
this.propID = undefined;
}

/**
* Given a resource property, check if an add button should be displayed under the property values
*
* @param prop the resource property
*/
addValueIsAllowed(prop: PropertyInfoValues): boolean {
const isAllowed = CardinalityUtil.createValueForPropertyAllowed(
prop.propDef.id, prop.values.length, this.parentResource.entityInfo.classes[this.parentResource.type]);

// check if:
// cardinality allows for a value to be added
// value component does not already have an add value form open
// user has write permissions
if (isAllowed && this.propID !== prop.propDef.id && this.addButtonIsVisible) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this do? this.propID !== prop.propDef.id

Copy link
Contributor

Choose a reason for hiding this comment

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

"value component does not already have an add value form open"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this.propID is a string that is assigned a prop.propDef.id of the corresponding property when the add button is clicked. This logic is used to hide the add button when the add value form for that property is already open so that the user can't keep opening new add value forms. Without this logic, the user would always see the add button under the property if the cardinality allows for it.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, I get it now :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

you can just return the whole expression, no need for an if

Copy link
Contributor Author

Choose a reason for hiding this comment

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

noob code fixed in 40cb19e

return true;
}

return false;
}
}