feat(shell-panel): enable all Position values for slotted Action Bar#13652
feat(shell-panel): enable all Position values for slotted Action Bar#13652brendan-vincent-rice wants to merge 79 commits intodevfrom
Position values for slotted Action Bar#13652Conversation
…, end, top, bottom) for slotted Action Bar
Position values (start, end, top, bottom) for slotted Action Bar
…tion-bar-position-property
…tion-bar-position-property
…tion-bar-position-property
…tion-bar-position-property
…osition-property' of https://github.com/Esri/calcite-design-system into brendan-vincent-rice/13559-shell-panel-add-action-bar-position-property
…tion-bar-position-property
…osition-property' of https://github.com/Esri/calcite-design-system into brendan-vincent-rice/13559-shell-panel-add-action-bar-position-property
brendan-vincent-rice
left a comment
There was a problem hiding this comment.
@eriklharper I refactored my changes to move them all to the bottom of the file, just so we know exactly what's being added.
We can make this easier to read and maintain by using nested :host selectors. If we remove Sass in the future, I think the syntax is a little different for native CSS, but still possible.
:host {
&([panel="start"][action-bar-position="bottom"]) {
...
&([resizable]) {
...
}
}
}
// Perhaps as part of a chore PR, we consider refactoring the whole file:
:host {
&([panel="start"] {
...
&([action-bar-position="bottom"]) {
...
&([resizable]) {
...
}
}
}
}
…tion-bar-position-property
…tion-bar-position-property
…osition-property' of https://github.com/Esri/calcite-design-system into brendan-vincent-rice/13559-shell-panel-add-action-bar-position-property
…tion-bar-position-property
…osition-property' of https://github.com/Esri/calcite-design-system into brendan-vincent-rice/13559-shell-panel-add-action-bar-position-property
| .container { | ||
| padding-inline-end: var(--calcite-internal-shell-panel-resize-handle-placeholder); | ||
| } |
There was a problem hiding this comment.
I feel like there is still a lot of duplication in this file. This rule, for instance, is repeated 3 times, so it would be nice if instead of 3 separate duplicate rules, we consolidate the selectors of each 3 into a single rule like:
:host([action-bar-position="end"][slot="panel-start"][resizable]),
:host([action-bar-position="top"][slot="panel-start"][resizable]),
:host([action-bar-position="bottom"][slot="panel-start"][resizable]) {
.container {
padding-inline-end: var(--calcite-internal-shell-panel-resize-handle-placeholder);
}
I'll peek at some other areas where I see unncessary duplication, but hopefully you get the idea. There are so many changes in this file, anything we can do to de-duplicate will make this code much more readable and maintainable in the future.
| .container { | ||
| flex-direction: column; | ||
| } |
There was a problem hiding this comment.
Here's another place where an internal css variable can assist. Rather than duplicating this selector/rule assignment in 10 places, let's create an internal variable called --calcite-internal-shell-panel-container-flex-direction and set that variable inside each of the 10 rulesets that it needs to be set and below each one of those you can simply do:
:host([action-bar-position="top"][slot="panel-start"]),
:host([action-bar-position="bottom"][slot="panel-start"]),
:host([action-bar-position="top"][slot="panel-end"]),
:host([action-bar-position="bottom"][slot="panel-end"]) {
--calcite-internal-shell-panel-container-flex-direction: column;
}
:host([action-bar-position="start"][slot="panel-bottom"]) {
--calcite-internal-shell-panel-container-flex-direction: row;
}
.... /* update all the other selectors to set this internal variable instead */
/* then at the very bottom you can simply do this: */
.container {
flex-direction: var(--calcite-internal-shell-panel-container-flex-direction);
}
| .action-bar-container { | ||
| overflow: hidden; | ||
| min-inline-size: 0; | ||
| flex: 0 1 auto; | ||
| } |
There was a problem hiding this comment.
This rule is also duplicated verbatim 12 times in this file. Is there a CSS state that these rules don't apply to? If not, this probably only needs to be declared once. If so, to reduce duplication you could invert the css selector to ignore the cases that don't apply using :not() since there seems to be many more places where this should apply than not apply, if that makes sense.
|
This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions. |
…tion-bar-position-property
Related Issue: #13559
Summary
This PR enables slotting of the Action Bar to into top, bottom, start, and end slots of the Shell Panel.