[ACS-5645] Property Panel Feature #3477
Conversation
Yasa-Nataliya
force-pushed
the
ACS-5645-property-panel-feature
branch
from
40211e2 to
59f541f
Compare
|
Please fix the circular dependency build issue as without it this PR and ADF part cannot be tested by reviewers |
projects/aca-shared/src/lib/components/info-drawer/info-drawer.component.ts
Outdated
Show resolved
Hide resolved
projects/aca-shared/src/lib/components/info-drawer/info-drawer.component.spec.ts
Outdated
Show resolved
Hide resolved
AnukritiGL
force-pushed
the
ACS-5645-property-panel-feature
branch
from
c0aa651 to
646175b
Compare
I have fixed the circular dependency issue. |
|
Build is passing locally after adding core and content-services from ADF
|
AnukritiGL
force-pushed
the
ACS-5645-property-panel-feature
branch
2 times, most recently
from
fa76268 to
969ee6b
Compare
| @@ -76,7 +81,13 @@ $defaults: ( | |||
| --theme-page-layout-header-background-color: $page-layout-header-background-color, | |||
| --theme-search-chip-icon-color: $search-chip-icon-color, | |||
| --theme-disabled-chip-background-color: $disabled-chip-background-color, | |||
| --theme-contrast-gray: $contrast-gray | |||
| --theme-contrast-gray: $contrast-gray, | |||
There was a problem hiding this comment.
this is already in the ADF, should not be redefining here
| @@ -36,6 +36,11 @@ $page-layout-header-background-color: #fff; | |||
| $search-chip-icon-color: #757575; | |||
| $disabled-chip-background-color: #f5f5f5; | |||
| $contrast-gray: #646569; | |||
| $metadata-property-panel-border-color: rgba(0, 0, 0, 0.12); | |||
There was a problem hiding this comment.
this is already defined in ADF
| @@ -361,4 +361,50 @@ export class ContentApiService { | |||
| requestVersionDirectAccessUrl(nodeId: string, versionId: string): Observable<DirectAccessUrlEntry> { | |||
| return from(this.versionsApi.requestDirectAccessUrl(nodeId, versionId)); | |||
| } | |||
|
|
|||
| getNodeIcon(node: Node): string { | |||
There was a problem hiding this comment.
this whole method is a wrapper for thumbnail service, it should be moved to thumbnail service instead
| @@ -111,6 +105,6 @@ export class MetadataTabComponent implements OnInit, OnDestroy { | |||
| } | |||
|
|
|||
| private checkIfNodeIsUpdatable(node: Node) { | |||
| this.canUpdateNode = node && !isLocked({ entry: node }) ? this.permission.check(node, ['update']) : false; | |||
| this.readOnly = !(node && !isLocked({ entry: node }) ? this.permission.check(node, ['update']) : false); | |||
There was a problem hiding this comment.
should be moved to metadata component where it checks for allowable operations
There was a problem hiding this comment.
Denys has changed this code. Not getting where should we move this exactly.
| border-radius: 4px; | ||
| margin-top: 12px; | ||
|
|
||
| &:focus-visible { |
There was a problem hiding this comment.
nested focus-visible is strange, what is it for?
| class="aca-close-details-button" | ||
| mat-icon-button | ||
| data-automation-id="close-library" | ||
| title="{{ 'APP.INFO_DRAWER.REDUCE_PANEL' | translate }}" |
There was a problem hiding this comment.
Are you sure that this button title should say reduce panel? In fact it's closing the panel so it sounds misleading
There was a problem hiding this comment.
@MichalKinas
Shane suggested the title should be 'Reduce Panel' so that we use the key with the same name.
Either we can change the key and keep the value same as Shane suggested. What do you think?
There was a problem hiding this comment.
If that title is fine with Shane it's okay for me as well
| getAllowedSidebarActions: jasmine.createSpy('getAllowedSidebarActions') | ||
| }; | ||
|
|
||
| const mockAspectActions = []; |
There was a problem hiding this comment.
Can you add the type here?
|
|
||
| const mockAspectActions = []; | ||
|
|
||
| const mockObservable = new BehaviorSubject(mockAspectActions); |
There was a problem hiding this comment.
Name of this variable doesn't sound correct, is suggests observable but in fact you're assigning the subject
There was a problem hiding this comment.
Also should not name be finished with $ as it is Subject?
| const mockAspectActions = []; | ||
|
|
||
| const mockObservable = new BehaviorSubject(mockAspectActions); | ||
| extensionsServiceMock.getAllowedSidebarActions.and.returnValue(mockObservable); |
There was a problem hiding this comment.
Are you sure this is correct? getAllowedSidebarActions expects Observable<Array<ContentActionRef>> instead of a subject
| const mockObservable = new BehaviorSubject(mockAspectActions); | ||
| extensionsServiceMock.getAllowedSidebarActions.and.returnValue(mockObservable); | ||
|
|
||
| const mockNode = { |
There was a problem hiding this comment.
Do you use all mocked properties? Also I think we should we use mocked node ids not real ones
| @@ -511,6 +511,16 @@ export const canEditAspects = (context: RuleContext): boolean => | |||
| repository.isMajorVersionAvailable(context, '7') | |||
| ].every(Boolean); | |||
|
|
|||
| export const editAspects = (context: RuleContext): boolean => | |||
| @@ -57,6 +57,38 @@ describe('InfoDrawerComponent', () => { | |||
| ]) | |||
| }; | |||
|
|
|||
| const mockNode = { | |||
There was a problem hiding this comment.
Same here for node ids and usage of defined properties
| let thumbnailService: ThumbnailService; | ||
| let service: any; | ||
|
|
||
| const mockNode = { |
There was a problem hiding this comment.
Same here for mocked node
|
|
||
| describe('Info Drawer header Icon', () => { | ||
| let thumbnailService: ThumbnailService; | ||
| let service: any; |
| }); | ||
|
|
||
| it('should resolve fallback file icon for unknown node', () => { | ||
| spyOn(thumbnailService, 'getDefaultMimeTypeIcon').and.returnValue(`assets/images/ft_ic_miscellaneous.svg`); |
There was a problem hiding this comment.
Shouldn't it use testNodeIcon as well?
projects/aca-content/src/lib/components/details/details.component.html
Outdated
Show resolved
Hide resolved
AnukritiGL
force-pushed
the
ACS-5645-property-panel-feature
branch
from
dddb6ec to
a31664e
Compare
|
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x")
What is the current behaviour? (You can also link to an open issue here)
In the property panel, there is an aspect icon at the bottom.
The design of the Property Panel appears outdated, and the title in the InfoDrawer is generic, labeled as "Details."
What is the new behaviour?
The Property Panel design has been revamped for both small and large screens. Additionally, the InfoDrawer now displays file titles with accompanying icons instead of the generic label "Details."
In accordance with the design, we removed the footer of the property panel and moved the edit aspect icon to the top.
Does this PR introduce a breaking change? (check one with "x")
If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...
Other information:
https://alfresco.atlassian.net/browse/ACS-5645
Based on requirements, updated the below points:
Title and Icon in property Panel: In the collapsed and expanded mode now displays file titles with accompanying icons instead of the generic label "Details."
Edit and Clear Button: As per design edit and clear button has been removed from the property panel fields.
Improved Styles: Worked on the styles of property panel. Panels now have a more appealing look, tailored to match figma design.
Dependent PR's
ADF: Alfresco/alfresco-ng2-components#8995
ADW: https://github.com/Alfresco/alfresco-applications/pull/459
ui-property.webm