[ACS-5540] Moved Aspects icons top of properties panel #3414
Conversation
projects/aca-content/src/lib/components/details/details.component.ts
Outdated
Show resolved
Hide resolved
| @@ -86,6 +92,8 @@ describe('DetailsComponent', () => { | |||
| fixture = TestBed.createComponent(DetailsComponent); | |||
| component = fixture.componentInstance; | |||
| contentApiService = TestBed.inject(ContentApiService); | |||
| mockNodeAspectService = TestBed.inject(NodeAspectService) as jasmine.SpyObj<NodeAspectService>; | |||
There was a problem hiding this comment.
this needs to be just a real service, you should not deal with mocks here
There was a problem hiding this comment.
updated with NodeAspectService.
projects/aca-content/src/lib/components/details/details.component.spec.ts
Outdated
Show resolved
Hide resolved
projects/aca-content/src/lib/components/details/details.component.ts
Outdated
Show resolved
Hide resolved
projects/aca-content/src/lib/components/details/details.component.ts
Outdated
Show resolved
Hide resolved
projects/aca-content/src/lib/components/info-drawer/metadata-tab/metadata-tab.component.ts
Outdated
Show resolved
Hide resolved
| this.isNodeLockedSubject.next(isLocked); | ||
| } | ||
|
|
||
| isNodeLocked(): Observable<boolean> { |
There was a problem hiding this comment.
this should be a public property isNodeLocked$ instead of a function
There was a problem hiding this comment.
please see other components and the use of observables
| --adf-metadata-property-panel-border-color: $adf-metadata-property-panel-border-color, | ||
| --adf-metadata-buttons-background-color: $adf-metadata-buttons-background-color, | ||
| --adf-snackbar-message-background-color: $adf-snackbar-message-background-color, | ||
| --adf-snackbar-message-color: $adf-snackbar-message-color |
There was a problem hiding this comment.
this does not seem right... the snackbar colors should already be defined elsewhere, and your PR should be focusing on aspects, not changing other component colors
There was a problem hiding this comment.
I have reverted snackbar colors
| @@ -117,13 +118,15 @@ export class ToggleEditOfflineComponent implements OnInit { | |||
| } | |||
|
|
|||
| lockNode(nodeId: string) { | |||
| this.nodeActionsService.setNodeLocked(true); | |||
There was a problem hiding this comment.
you should all this only when "nodesApi.lockNode" is complete
There was a problem hiding this comment.
we are tried this aproach
lockNode(nodeId: string) {
let nodeToBeLocked;
this.nodesApi.unlockNode(nodeId)
.then(() => {
nodeToBeLocked = this.nodeActionsService.setNodeLocked(true);
})
.catch((error) => {
throw new Error(Locking failed - ${error});
});
return nodeToBeLocked;
}
but this not working properly for locking and unlocking the file. so we will take this along wth bug fixes.
There was a problem hiding this comment.
if you need extra bug fixes after merging this PR then please add those bug fixes here, as the PR is not a clean implementation then
There was a problem hiding this comment.
All bugs in the new design have been fixed
| return this.nodesApi.lockNode(nodeId, { | ||
| type: 'ALLOW_OWNER_CHANGES', | ||
| lifetime: 'PERSISTENT' | ||
| }); | ||
| } | ||
|
|
||
| unlockNode(nodeId: string) { | ||
| this.nodeActionsService.setNodeLocked(false); |
There was a problem hiding this comment.
should be called only when "unlockNode" is complete, use "subscribe" or "then"
There was a problem hiding this comment.
| @@ -117,13 +118,15 @@ export class ToggleEditOfflineComponent implements OnInit { | |||
| } | |||
|
|
|||
| lockNode(nodeId: string) { | |||
| this.nodeActionsService.setNodeLocked(true); | |||
There was a problem hiding this comment.
if you need extra bug fixes after merging this PR then please add those bug fixes here, as the PR is not a clean implementation then
Yasa-Nataliya
force-pushed
the
ACS-5540
branch
from
c94e059 to
a360aae
Compare
Yasa-Nataliya
force-pushed
the
ACS-5540
branch
2 times, most recently
from
cac2d13 to
2ece2fb
Compare
Yasa-Nataliya
force-pushed
the
ACS-5540
branch
from
3621750 to
7462f25
Compare
| @Input() | ||
| node: Node; | ||
|
|
||
| displayAspect$: Observable<string>; | ||
| canUpdateNode = false; | ||
| editable = false; | ||
| editableTags = false; | ||
| editableCategories = false; | ||
| group: any = { |
There was a problem hiding this comment.
can you avoid using any?
There was a problem hiding this comment.
I will update this once ADF upstream PR merges.
There was a problem hiding this comment.
ok please correct it after ADF upstream PR is merged and before you merge that ACA PR.
Yasa-Nataliya
force-pushed
the
ACS-5540
branch
from
0d43a45 to
ef739c0
Compare
|
@Yasa-Nataliya I will approve that PR whenever last comment #3414 (comment) (regarding to correct type) will be addressed so whenever you merge ADF upstream and correct that type in this ACA PR after merging upstream as rest my comments are addressed. |
|
|
||
| &:focus { | ||
| background-color: var(--theme-selected-background-color); | ||
| outline: 2px solid var(--theme-blue-button-color); |
There was a problem hiding this comment.
Some focus styling are not working as expected
There was a problem hiding this comment.
Same for close and save buttons for each section
There was a problem hiding this comment.
We have fixed the focus issue.
Yasa-Nataliya
force-pushed
the
ACS-5540
branch
from
cd0a67a to
0559405
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.
What is the new behaviour?
In accordance with the design, we removed the footer of the property panel and moved the edit aspect icon to the top.
screen-capture (16).webm
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-5540
Alfresco/alfresco-ng2-components#8837 was a dependency of this PR.