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.

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(dialog-wrapper): update heading to fully span grid area #4810

Merged
merged 9 commits into from
Oct 17, 2024
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ executors:
parameters:
current_golden_images_hash:
type: string
default: 3bb8a93772e35ef3bb5e156f76b7c8bd72786144
default: ef7c7c0e0b5827a2f5e95bf6aba539959453f5dc
wireit_cache_name:
type: string
default: wireit
Expand Down
20 changes: 20 additions & 0 deletions packages/dialog/src/Dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ export class Dialog extends ObserveSlotPresence(AlertDialog, [
@query('.close-button')
closeButton?: CloseButton;

/**
* @deprecated Use the Alert Dialog component with `variant="error"` instead.
*/
@property({ type: Boolean, reflect: true })
public error = false;

Expand Down Expand Up @@ -170,4 +173,21 @@ export class Dialog extends ObserveSlotPresence(AlertDialog, [
super.firstUpdated(changes);
this.setAttribute('role', 'dialog');
}

protected override updated(changes: PropertyValues): void {
super.updated(changes);
if (
changes.has('error') &&
(this.error !== undefined || changes.get('error') !== undefined)
) {
if (window.__swc.DEBUG) {
window.__swc.warn(
this,
`The "error" attribute of <${this.localName}> has been deprecated. Use the Alert Dialog component with the "variant='error'" instead. "error" will be removed in a future release.`,
'https://opensource.adobe.com/spectrum-web-components/components/alert-dialog/#error',
{ level: 'deprecation' }
);
}
}
}
}
21 changes: 21 additions & 0 deletions packages/dialog/src/DialogWrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
CSSResultArray,
html,
nothing,
PropertyValues,
TemplateResult,
} from '@spectrum-web-components/base';
import { property } from '@spectrum-web-components/base/src/decorators.js';
Expand Down Expand Up @@ -41,6 +42,9 @@ export class DialogWrapper extends DialogBase {
return [...super.styles];
}

/**
* @deprecated Use the Alert Dialog component with `variant="error"` instead.
*/
@property({ type: Boolean, reflect: true })
public error = false;

Expand Down Expand Up @@ -105,6 +109,23 @@ export class DialogWrapper extends DialogBase {
);
}

protected override updated(changes: PropertyValues<this>): void {
super.updated(changes);
if (
changes.has('error') &&
(this.error !== undefined || changes.get('error') !== undefined)
) {
if (window.__swc.DEBUG) {
window.__swc.warn(
this,
`The "error" attribute of <${this.localName}> has been deprecated. Use the Alert Dialog component with the "variant='error'" instead. "error" will be removed in a future release.`,
'https://opensource.adobe.com/spectrum-web-components/components/alert-dialog/#error',
{ level: 'deprecation' }
);
}
}
}

protected override renderDialog(): TemplateResult {
const hideDivider =
this.noDivider ||
Expand Down
17 changes: 16 additions & 1 deletion packages/dialog/src/dialog.css
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,26 @@ governing permissions and limitations under the License.
height: auto;
}

/* We may remove this override when CSS fixes unused `header` and `typeIcon` areas */
.grid {
grid-template-areas:
'hero hero hero hero hero hero'
'. . . . . .'
'. heading header header typeIcon .'
'. heading heading heading typeIcon . '
'. divider divider divider divider .'
'. content content content content .'
'. footer footer buttonGroup buttonGroup .'
'. . . . . .';
inline-size: 100%;
display: grid;
}

/* Needed while `error` attribute still exists as deprecated */
:host(:not([error], [dismissable], [mode])) .grid {
grid-template-areas:
'hero hero hero hero hero hero'
'. . . . . .'
'. heading heading heading heading . '
'. divider divider divider divider .'
'. content content content content .'
'. footer footer buttonGroup buttonGroup .'
Expand Down
32 changes: 32 additions & 0 deletions packages/dialog/stories/dialog-wrapper.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,38 @@ export const longContent = (

longContent.decorators = [isOverlayOpen];

export const longHeading = (
args: StoryArgs = {},
context: { viewMode?: string } = {}
): TemplateResult => {
const open = context.viewMode === 'docs' ? undefined : 'click';
return html`
<overlay-trigger
type="modal"
@close=${handleClose(args)}
open=${ifDefined(open)}
>
<sp-dialog-wrapper
slot="click-content"
underlay
headline="Dialog long long long long long long long long long long long long title"
confirm-label="Keep Both"
secondary-label="Replace"
cancel-label="Cancel"
footer="Content for footer"
size="m"
>
Content of the dialog
</sp-dialog-wrapper>
<sp-button slot="trigger" variant="primary">
Toggle Dialog
</sp-button>
</overlay-trigger>
`;
};

longHeading.decorators = [isOverlayOpen];

export const wrapperDismissableUnderlayError = (
args: StoryArgs = {},
context: { viewMode?: string } = {}
Expand Down
53 changes: 53 additions & 0 deletions packages/dialog/test/dialog-wrapper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -368,3 +368,56 @@ describe('Dialog Wrapper', () => {
expect(contentElement.hasAttribute('tabindex')).to.be.true;
});
});

describe('dev mode', () => {
let consoleWarnStub!: ReturnType<typeof stub>;
before(() => {
window.__swc.verbose = true;
consoleWarnStub = stub(console, 'warn');
});
afterEach(() => {
consoleWarnStub.resetHistory();
});
after(() => {
window.__swc.verbose = false;
consoleWarnStub.restore();
});

it('warns that `error` is deprecated', async () => {
const el = await styledFixture<DialogWrapper>(
wrapperDismissableUnderlayError()
);
await elementUpdated(el);

expect(consoleWarnStub.called).to.be.true;
const dialogWrapperSpyCall = consoleWarnStub.getCall(1);
const dialogSpyCall = consoleWarnStub.getCall(2);
expect(
(dialogWrapperSpyCall.args.at(0) as string).includes('"error"'),
'confirm error-centric message in dialog wrapper'
).to.be.true;
expect(
dialogWrapperSpyCall.args.at(-1),
'confirm `data` shape'
).to.deep.equal({
data: {
localName: 'sp-dialog-wrapper',
type: 'api',
level: 'deprecation',
},
});
expect(
(dialogSpyCall.args.at(0) as string).includes('"error"'),
'confirm error-centric message in extended dialog class'
).to.be.true;
expect(dialogSpyCall.args.at(-1), 'confirm `data` shape').to.deep.equal(
{
data: {
localName: 'sp-dialog',
type: 'api',
level: 'deprecation',
},
}
);
});
});
85 changes: 54 additions & 31 deletions packages/dialog/test/dialog.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
fullscreen,
small,
} from '../stories/dialog.stories.js';
import { spy } from 'sinon';
import { spy, stub } from 'sinon';
import { testForLitDevWarnings } from '../../../test/testing-helpers.js';

describe('Dialog', () => {
Expand Down Expand Up @@ -156,11 +156,9 @@ describe('Dialog', () => {

customElements.define('hero-dialog', Override);

const el = await fixture<Override>(
html`
<hero-dialog></hero-dialog>
`
);
const el = await fixture<Override>(html`
<hero-dialog></hero-dialog>
`);

const container = el.shadowRoot.querySelector('#hero-container');
expect(container).to.not.be.null;
Expand All @@ -176,11 +174,9 @@ describe('Dialog', () => {

customElements.define('heading-dialog', Override);

const el = await fixture<Override>(
html`
<heading-dialog></heading-dialog>
`
);
const el = await fixture<Override>(html`
<heading-dialog></heading-dialog>
`);

const container = el.shadowRoot.querySelector('#heading-container');
expect(container).to.not.be.null;
Expand All @@ -196,11 +192,9 @@ describe('Dialog', () => {

customElements.define('content-dialog', Override);

const el = await fixture<Override>(
html`
<content-dialog></content-dialog>
`
);
const el = await fixture<Override>(html`
<content-dialog></content-dialog>
`);

const container = el.shadowRoot.querySelector('#content-container');
expect(container).to.not.be.null;
Expand All @@ -219,11 +213,9 @@ describe('Dialog', () => {

customElements.define('footer-dialog', Override);

const el = await fixture<Override>(
html`
<footer-dialog></footer-dialog>
`
);
const el = await fixture<Override>(html`
<footer-dialog></footer-dialog>
`);

const container = el.shadowRoot.querySelector('#footer-container');
expect(container).to.not.be.null;
Expand All @@ -242,11 +234,9 @@ describe('Dialog', () => {

customElements.define('button-dialog', Override);

const el = await fixture<Override>(
html`
<button-dialog></button-dialog>
`
);
const el = await fixture<Override>(html`
<button-dialog></button-dialog>
`);

const container = el.shadowRoot.querySelector('#button-container');
expect(container).to.not.be.null;
Expand All @@ -262,13 +252,46 @@ describe('Dialog', () => {

customElements.define('dismiss-dialog', Override);

const el = await fixture<Override>(
html`
<dismiss-dialog dismissable></dismiss-dialog>
`
);
const el = await fixture<Override>(html`
<dismiss-dialog dismissable></dismiss-dialog>
`);

const container = el.shadowRoot.querySelector('#dismiss-container');
expect(container).to.not.be.null;
});
});

describe('dev mode', () => {
let consoleWarnStub!: ReturnType<typeof stub>;
before(() => {
window.__swc.verbose = true;
consoleWarnStub = stub(console, 'warn');
});
afterEach(() => {
consoleWarnStub.resetHistory();
});
after(() => {
window.__swc.verbose = false;
consoleWarnStub.restore();
});

it('warns that `error` is deprecated', async () => {
const el = await fixture<Dialog>(alertError());

await elementUpdated(el);

expect(consoleWarnStub.called).to.be.true;
const spyCall = consoleWarnStub.getCall(0);
expect(
(spyCall.args.at(0) as string).includes('"error"'),
'confirm error-centric message'
).to.be.true;
expect(spyCall.args.at(-1), 'confirm `data` shape').to.deep.equal({
data: {
localName: 'sp-dialog',
type: 'api',
level: 'deprecation',
},
});
});
});
Loading