Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(panel): Remove double border styling when content isn't provided #7368

Merged
merged 15 commits into from
Aug 2, 2023
Merged

fix(panel): Remove double border styling when content isn't provided #7368

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
8fe2141
fix(panel): Remove double border styling when a footer is present wit…
driskull Jul 24, 2023
4eca82f
fix borders
driskull Jul 24, 2023
18b6399
cleanup
driskull Jul 24, 2023
c44eaf7
cleanup
driskull Jul 25, 2023
c72b469
cleanup
driskull Jul 25, 2023
27e11d5
cleanup
driskull Jul 25, 2023
38debdf
Merge branch 'main' into dris0000/panel-footer-without-content
driskull Jul 25, 2023
8b9ee32
story update
driskull Jul 25, 2023
862c543
Merge branch 'dris0000/panel-footer-without-content' of github.com:Es…
driskull Jul 25, 2023
3250ea2
Update panel.tsx
driskull Jul 25, 2023
14d2a7c
cleanup
driskull Jul 25, 2023
144edd6
Merge branch 'main' into dris0000/panel-footer-without-content
driskull Jul 25, 2023
6ae14e4
refactoring
driskull Jul 25, 2023
1d97835
Merge branch 'dris0000/panel-footer-without-content' of github.com:Es…
driskull Jul 25, 2023
dddf440
rename class
driskull Aug 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 8 additions & 11 deletions packages/calcite-components/src/components/panel/panel.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,8 @@
}

.header {
border-block-end: 1px solid;
border-block-end: 1px solid var(--calcite-ui-border-3);
Copy link
Member Author

Choose a reason for hiding this comment

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

moved these together since border-block-end isnt exactly the same as border-bottom. Applies to other CSS

@apply bg-foreground-1
border-b-color-3
w-full
items-stretch
justify-start
Expand All @@ -40,9 +39,9 @@
}

.action-bar-container {
border-block-end: 1px solid;
border-block-end: 1px solid var(--calcite-ui-border-3);
@apply w-full
z-header border-b-color-3;
z-header;
}

.action-bar-container ::slotted(calcite-action-bar) {
Expand Down Expand Up @@ -92,11 +91,7 @@
}

.content-wrapper {
@apply overflow-auto;
}

.content-height {
@apply h-full;
@apply overflow-auto h-full;
}

.content-container {
Expand All @@ -109,9 +104,7 @@
}

.footer {
border-block-start: 1px solid;
@apply bg-foreground-1
border-t-color-3
flex
w-full
justify-evenly;
Expand All @@ -120,6 +113,10 @@
padding: var(--calcite-panel-footer-padding);
}

.footer-border {
border-block-start: 1px solid var(--calcite-ui-border-3);
}

.fab-container {
@apply sticky
bottom-0
Expand Down
8 changes: 8 additions & 0 deletions packages/calcite-components/src/components/panel/panel.stories.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,3 +218,11 @@ export const actionBarBackgroundColor_TestOnly = (): string => html`<calcite-pan
<p style="height: 400px">Hello world!</p>
<p slot="footer">Slotted content!</p>
</calcite-panel>`;

export const footerWithoutContent_TestOnly = (): string => html`<calcite-panel
height-scale="s"
heading="Header!"
style="width: 300px; height:auto;"
>
<p slot="footer">Footer content!</p>
</calcite-panel>`;
81 changes: 28 additions & 53 deletions packages/calcite-components/src/components/panel/panel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ import {
VNode,
Watch,
} from "@stencil/core";
import { focusFirstTabbable, slotChangeGetAssignedElements, toAriaBoolean } from "../../utils/dom";
import {
focusFirstTabbable,
slotChangeGetAssignedElements,
slotChangeHasAssignedElement,
toAriaBoolean,
} from "../../utils/dom";
import {
connectInteractive,
disconnectInteractive,
Expand Down Expand Up @@ -168,6 +173,8 @@ export class Panel

resizeObserver = createObserver("resize", () => this.resizeHandler());

@State() hasDefaultContent = false;

@State() hasStartActions = false;

@State() hasEndActions = false;
Expand Down Expand Up @@ -257,28 +264,20 @@ export class Panel
this.calcitePanelScroll.emit();
};

handleHeaderActionsStartSlotChange = (event: Event): void => {
const elements = (event.target as HTMLSlotElement).assignedElements({
flatten: true,
});
handleDefaultSlotChange = (event: Event): void => {
this.hasDefaultContent = slotChangeHasAssignedElement(event);
};

this.hasStartActions = !!elements.length;
handleHeaderActionsStartSlotChange = (event: Event): void => {
this.hasStartActions = slotChangeHasAssignedElement(event);
};

handleHeaderActionsEndSlotChange = (event: Event): void => {
const elements = (event.target as HTMLSlotElement).assignedElements({
flatten: true,
});

this.hasEndActions = !!elements.length;
this.hasEndActions = slotChangeHasAssignedElement(event);
};

handleHeaderMenuActionsSlotChange = (event: Event): void => {
const elements = (event.target as HTMLSlotElement).assignedElements({
flatten: true,
});

this.hasMenuItems = !!elements.length;
this.hasMenuItems = slotChangeHasAssignedElement(event);
};

handleActionBarSlotChange = (event: Event): void => {
Expand All @@ -292,35 +291,19 @@ export class Panel
};

handleHeaderContentSlotChange = (event: Event): void => {
const elements = (event.target as HTMLSlotElement).assignedElements({
flatten: true,
});

this.hasHeaderContent = !!elements.length;
this.hasHeaderContent = slotChangeHasAssignedElement(event);
};

handleFooterSlotChange = (event: Event): void => {
const elements = (event.target as HTMLSlotElement).assignedElements({
flatten: true,
});

this.hasFooterContent = !!elements.length;
this.hasFooterContent = slotChangeHasAssignedElement(event);
};

handleFooterActionsSlotChange = (event: Event): void => {
const elements = (event.target as HTMLSlotElement).assignedElements({
flatten: true,
});

this.hasFooterActions = !!elements.length;
this.hasFooterActions = slotChangeHasAssignedElement(event);
};

handleFabSlotChange = (event: Event): void => {
const elements = (event.target as HTMLSlotElement).assignedElements({
flatten: true,
});

this.hasFab = !!elements.length;
this.hasFab = slotChangeHasAssignedElement(event);
};

// --------------------------------------------------------------------------
Expand Down Expand Up @@ -498,12 +481,15 @@ export class Panel
}

renderFooterNode(): VNode {
const { hasFooterContent, hasFooterActions } = this;
const { hasFooterContent, hasFooterActions, hasDefaultContent } = this;

const showFooter = hasFooterContent || hasFooterActions;

return (
<footer class={CSS.footer} hidden={!showFooter}>
<footer
class={{ [CSS.footer]: true, [CSS.footerBorder]: hasDefaultContent }}
hidden={!showFooter}
>
<slot key="footer-slot" name={SLOTS.footer} onSlotchange={this.handleFooterSlotChange} />
<slot
key="footer-actions-slot"
Expand All @@ -525,27 +511,16 @@ export class Panel
};

renderContent(): VNode {
const { hasFab } = this;

const defaultSlotNode: VNode = <slot key="default-slot" />;
const containerNode = hasFab ? (
<section class={CSS.contentContainer}>{defaultSlotNode}</section>
Copy link
Member Author

Choose a reason for hiding this comment

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

@asangma do you think you could review these changes? I changed this because it was causing some problems with onSlotChange since this default slot was getting moved into a section element depending on if there was a FAB or not. I don't think its necessary if we always have the section element present and the FAB content classes are no longer conditional.

) : (
defaultSlotNode
);

return (
<div
class={{
[CSS.contentWrapper]: true,
[CSS.contentContainer]: !hasFab,
[CSS.contentHeight]: hasFab,
}}
class={CSS.contentWrapper}
onScroll={this.panelScrollHandler}
// eslint-disable-next-line react/jsx-sort-props
ref={this.setPanelScrollEl}
>
{containerNode}
<section class={CSS.contentContainer}>
Copy link
Member Author

Choose a reason for hiding this comment

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

@asangma do you remember why this section was necessary for the FAB? Is there any harm in always having it there for consistency and so the default slot doesn't get rearranged?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the section was there to keep the FAB sticky. But if it can be sticky without the section, that's cool too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok. i have it so the section is always there regardless of the FAB or not. Ill move forward with that since its safer.

<slot onSlotchange={this.handleDefaultSlotChange} />
</section>
{this.renderFab()}
</div>
);
Expand Down
2 changes: 1 addition & 1 deletion packages/calcite-components/src/components/panel/resources.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ export const CSS = {
headerActionsStart: "header-actions--start",
contentWrapper: "content-wrapper",
contentContainer: "content-container",
contentHeight: "content-height",
fabContainer: "fab-container",
footer: "footer",
footerBorder: "footer-border",
};

export const ICONS = {
Expand Down