Skip to content

Commit

Permalink
fix(dialog-wrapper): update heading to fully span grid area (#4810)
Browse files Browse the repository at this point in the history
  • Loading branch information
rubencarvalho authored Oct 17, 2024
1 parent 088e84c commit 7d1f461
Show file tree
Hide file tree
Showing 7 changed files with 197 additions and 33 deletions.
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',
},
});
});
});

0 comments on commit 7d1f461

Please sign in to comment.