files: add permissions #555
Conversation
jma
force-pushed
the
maj-add-file-permissions
branch
2 times, most recently
from
b297e93 to
f8fc738
Compare
| <div class="card-footer text-center"> | ||
| <button class="btn btn-sm btn-primary" (click)="manageFile(null)"> | ||
| <div *ngIf=canAdd.can class="card-footer text-center"> | ||
| <button class="btn btn-sm btn-primary" *ngIf=canAdd (click)="manageFile(null)"> |
There was a problem hiding this comment.
Is it necessary to have *ngIf=canAdd because there's already a div definition on the previous line?
| @@ -484,9 +474,12 @@ export class RecordFilesComponent implements OnDestroy, OnInit { | |||
| return this._fileService.list(this.type, this.pid); | |||
| }), | |||
| tap((files) => { | |||
| console.log('tap1'); | |||
There was a problem hiding this comment.
most of my comments are suggestions.
I'm confused about the huge usage of Bootstrap classes into ng-core component : It doesn't allow child projects to override them.
Instead of <div class="ml-2 py-2 font-weight-bold text-primary border-bottom"> we should use <div class="myComponentSpecialClass"> and apply default style on this class.
There was a problem hiding this comment.
you could delete this file
| @@ -0,0 +1,64 @@ | |||
| <<ng-container *ngIf="file"> | |||
There was a problem hiding this comment.
remove extra < character
| @@ -0,0 +1,64 @@ | |||
| <<ng-container *ngIf="file"> | |||
| <div class="row"> | |||
| <div class="col-sm-6" [ngClass]="{ 'pl-5 text-muted': !file.is_head }"> | |||
There was a problem hiding this comment.
should be simply col-6 or maybe better, simply col (because each row seems contain 2 same-sized columns)
| </div> | ||
| </div> | ||
| <div class="row mt-2" *ngIf="file.showInfo"> | ||
| <div class="col-lg-6" *ngIf="file.is_head"> |
There was a problem hiding this comment.
same than previous comment.
Try to be consitent between "sm", "md" and "lg" usage. Here you have a parent div with "lg" but some children with "sm"
| <dl class="row mt-2 mb-0"> | ||
| <ng-container *ngFor="let item of file.metadata | keyvalue"> | ||
| <ng-container *ngIf="!infoExcludedFields.includes(item.key) && item.value"> | ||
| <dt class="col-sm-4">{{ item.key | translate }}</dt> | ||
| <dd class="col-sm-8">{{ item.value | translate }}</dd> | ||
| </ng-container> | ||
| </ng-container> | ||
| </dl> |
There was a problem hiding this comment.
No need to specify the size of last column of the row in this case. You could write
<dl class="row">
<dt class="col-sm-6 col-md-4">key</dt>
<dd class="col">value</dt>
</dl>in such case, the width of the col will be adapted regarding free space on the row
| <div class="col-lg-6" [ngClass]="{ 'pl-5 text-muted': !file.is_head }"> | ||
| <dl class="row mt-2 mb-0"> | ||
| <dt class="col-sm-4" translate>Size</dt> | ||
| <dd class="col-sm-8">{{ file.size | filesize }}</dd> | ||
|
|
||
| <dt class="col-sm-4" translate>Mime type</dt> | ||
| <dd class="col-sm-8">{{ file.mimetype }}</dd> | ||
|
|
||
| <dt class="col-sm-4" translate>Checksum</dt> | ||
| <dd class="col-sm-8">{{ file.checksum }}</dd> | ||
|
|
||
| <dt class="col-sm-4" translate>Modified at</dt> | ||
| <dd class="col-sm-8">{{ file.updated | dateTranslate : 'medium' }}</dd> | ||
| </dl> | ||
| </div> |
There was a problem hiding this comment.
exactly the same + mix between "lg" and "sm"
| @Input() | ||
| record: any; |
There was a problem hiding this comment.
as @Output are in single line, all @Input could be too
| <i class="fa fa-plus mr-1"></i>{{ 'Add a new file' | translate }} | ||
| </button> | ||
| </div> | ||
| </div> | ||
|
|
||
| <ng-template #formModal> | ||
| <div class="modal-header"> | ||
| <h4 class="modal-title pull-left">{{ currentFile ? currentFile.key : 'Add a new file' | translate }}</h4> | ||
| <h4 class="modal-title pull-left">{{ currentFile ? currentFile.key : ('Add a new file' | translate) }}</h4> |
| canAdd$.subscribe((result: ActionStatus) => { | ||
| this.canAdd = result; | ||
| }) |
There was a problem hiding this comment.
single instruction, could removed {} --> canAdd$.subscribe((result: ActionStatus) => this.canAdd = result)
jma
force-pushed
the
maj-add-file-permissions
branch
from
f8fc738 to
59bd9ad
Compare
* Fixes broken `ng` command such as `ng c`. * Adds permissions support to the file component to hide action buttons. Co-Authored-by: Johnny Mariéthoz <Johnny.Mariethoz@rero.ch>
jma
force-pushed
the
maj-add-file-permissions
branch
from
59bd9ad to
6651c91
Compare
ngcommand such asng c.Co-Authored-by: Johnny Mariéthoz Johnny.Mariethoz@rero.ch
Why are you opening this PR?
How to test?
Code review check list