Skip to content

feat(shell-panel): enable all Position values for slotted Action Bar#13652

Open
brendan-vincent-rice wants to merge 79 commits intodevfrom
brendan-vincent-rice/13559-shell-panel-add-action-bar-position-property
Open

feat(shell-panel): enable all Position values for slotted Action Bar#13652
brendan-vincent-rice wants to merge 79 commits intodevfrom
brendan-vincent-rice/13559-shell-panel-add-action-bar-position-property

Conversation

@brendan-vincent-rice
Copy link
Copy Markdown
Member

@brendan-vincent-rice brendan-vincent-rice commented Jan 7, 2026

Related Issue: #13559

Summary

This PR enables slotting of the Action Bar to into top, bottom, start, and end slots of the Shell Panel.

@brendan-vincent-rice brendan-vincent-rice changed the title Brendan vincent rice/13559 shell panel add action bar position property feat(shell-panel): enable all Position values (start, end, top, bottom) for slotted Action Bar Jan 7, 2026
@github-actions github-actions Bot added the enhancement Issues tied to a new feature or request. label Jan 7, 2026
Comment thread packages/components/src/components/shell-panel/shell-panel.scss Outdated
Comment thread packages/components/src/components/shell-panel/shell-panel.scss Outdated
Comment thread packages/components/src/components/shell-panel/shell-panel.scss Outdated
Comment thread packages/components/src/components/shell-panel/shell-panel.scss
Comment thread packages/components/src/components/shell-panel/shell-panel.scss Outdated
Copy link
Copy Markdown
Member Author

@brendan-vincent-rice brendan-vincent-rice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eriklharper I refactored my changes to move them all to the bottom of the file, just so we know exactly what's being added.

Image

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]) {
          ...
      }
    }
  }
}

Comment thread packages/components/index.html Outdated
Comment thread packages/components/index.html Outdated
@brendan-vincent-rice brendan-vincent-rice added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Mar 19, 2026
@brendan-vincent-rice brendan-vincent-rice added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Mar 19, 2026
@brendan-vincent-rice brendan-vincent-rice added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Mar 20, 2026
@eriklharper eriklharper requested a review from jcfranco March 20, 2026 19:42
Comment on lines +630 to +632
.container {
padding-inline-end: var(--calcite-internal-shell-panel-resize-handle-placeholder);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +604 to +606
.container {
flex-direction: column;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
}

Comment on lines +608 to +612
.action-bar-container {
overflow: hidden;
min-inline-size: 0;
flex: 0 1 auto;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 4, 2026

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.

@github-actions github-actions Bot added the Stale Issues or pull requests that have not had recent activity. label Apr 4, 2026
@brendan-vincent-rice brendan-vincent-rice added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed Stale Issues or pull requests that have not had recent activity. pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Apr 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Issues tied to a new feature or request. pr ready for visual snapshots Adding this label will run visual snapshot testing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants