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
feat(lists): read mode project member (DEV-1343) #825
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -2,28 +2,28 @@ | |||
<div class="list-node" *ngFor="let node of list; let first = first; let last = last;"> | |||
|
|||
<!-- button to expand / close node --> | |||
<button type="button" mat-icon-button (click)="toggleChildren(node.id)" class=""> | |||
<button *ngIf="node.children.length !== 0" type="button" mat-icon-button (click)="toggleChildren(node.id)"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This causes the arrow to be hidden even for project admins. The arrow needs to be shown for project admins so that they can add child node even if one doesn't already exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in cd87150
<mat-icon class="mat-icon-rtl-mirror"> | ||
{{showChildren(node.id) ? 'expand_more' : 'chevron_right'}} | ||
</mat-icon> | ||
</button> | ||
|
||
<div class="list-item-indentation" *ngIf="node.children.length === 0"></div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of doing this, I would add a conditional class on the button above in the event that the node has no children. Something like [class.list-item-indentation]="node.children.length === 0".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried with the class condition but couldn't do it. So I changed the whole situation by adding a div around the button with a certain size -> cd87150
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a small thing and then I will approve :)
projectAdmin = false; | ||
|
||
// project data | ||
project: ReadProject; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this variable is needed, you can use a const
to temporarily store the project you get from the cache
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, it is not needed elsewhere (ff8b5a8)
// get the project data from cache | ||
this._cache.get(this.projectCode).subscribe( | ||
(response: ReadProject) => { | ||
this.project = response; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use a const
here instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would go further and not even use const
but pass the response
directly because it is not needed elsewhere (ff8b5a8)
projectAdmin = false; | ||
|
||
// project data | ||
project: ReadProject; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same thing here as in the list-item-form
component
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -26,8 +26,10 @@ import { OntologyService } from './ontology/ontology.service'; | |||
styleUrls: ['./project.component.scss'] | |||
}) | |||
export class ProjectComponent implements OnInit { | |||
readonly TAB_DATA_MODEL = 3; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works well, thanks!
resolves DEV-1343
The lists (within the list tab and on the sidenav) are visible for project member but all the CRUD operations regarding lists were hidden.