feat(side-panel): extract status actions into new component#1676
feat(side-panel): extract status actions into new component#1676
Conversation
There was a problem hiding this comment.
Code Review
This pull request is a great improvement, extracting the status actions into new standalone components. This refactoring simplifies the API for developers and allows for more flexible use of features like overlays, while also deprecating the older StatusItem interface. The implementation largely adheres to the repository's style guide. I've identified a minor opportunity for improvement in the SCSS to avoid using !important, which will enhance maintainability.
5dd4e5e to
d367492
Compare
spliffone
left a comment
There was a problem hiding this comment.
I would prefer an approach where we just use content-projection and some CSS classes to show status actions which offers more flexibility for consumers. The status action components has simply to much logic inside.
projects/element-ng/side-panel/si-side-panel-action.component.ts
Outdated
Show resolved
Hide resolved
4e7ca76 to
846a27d
Compare
|
@spliffone I chose a component here to maintain some flexibility for us in the future. On the long run, I would like to have a more flexible solution to render icons / actions in a collapsed side-panel (needed for AI patterns). But currently, the main focus is solve a consumer request. |
4512f3f to
32c49c0
Compare
ljanner
left a comment
There was a problem hiding this comment.
@spike-rabbit great changes! I really like the clear naming and good examples.
Just two small details. The commit body already gives great insight to the change, but please add a deprecation note so it properly shows up in the changelog.
projects/element-ng/side-panel/si-side-panel-action.component.ts
Outdated
Show resolved
Hide resolved
projects/element-ng/side-panel/si-side-panel-action.component.ts
Outdated
Show resolved
Hide resolved
projects/element-ng/side-panel/si-side-panel-action.component.scss
Outdated
Show resolved
Hide resolved
projects/element-ng/side-panel/si-side-panel-action.component.ts
Outdated
Show resolved
Hide resolved
32c49c0 to
0bfffdc
Compare
d9fa945 to
0d8f545
Compare
0d8f545 to
e0e3a53
Compare
e0e3a53 to
a879f43
Compare
a879f43 to
dca0389
Compare
The main motivation is to allow access to the native elements.
In some applications a status action triggers a menu or overlay.
This is currently built by those apps using a DOM query.
With this change, they can just use the overlay directive.
In addition, we can deprecate the `StatusItem` interface,
which depends on the already deprecated `MenuItem`.
The usage is quite simple:
```html
<si-side-panel-content>
<si-side-panel-actions>
<button
type="button"
si-side-panel-action
icon="element-alarm-background-filled"
iconColor="status-danger"
stackedIcon="element-alarm-tick"
stackedIconColor="text-body"
(click)="action()"
>
Action
</button>
</si-side-panel-actions>
</si-side-panel-content>
```
DEPRECATED:
The input `SiSidePanelContentComponent.statusActions` should no longer be used.
Use the new `<si-side-panel-actions>` instead:
```html
<si-side-panel-content>
<si-side-panel-actions>
<button
type="button"
si-side-panel-action
icon="element-alarm-background-filled"
iconColor="status-danger"
stackedIcon="element-alarm-tick"
stackedIconColor="text-body"
(click)="action()"
>
Action
</button>
</si-side-panel-actions>
</si-side-panel-content>
```
dca0389 to
65e234c
Compare
ljanner
left a comment
There was a problem hiding this comment.
Now code owners should work 😉
The main motivation is to allow access to the native elements.
In some applications a status action triggers a menu or overlay.
This is currently built by those apps using a DOM query.
With this change, they can just use the overlay directive.
In addition, we can deprecate the
StatusIteminterface,which depends on the already deprecated
MenuItem.The usage is quite simple:
Documentation.
Examples.
Dashboards Demo.
Playwright report.
Coverage Reports: