Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .vdiff.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"browsers": [
{
"name": "Chromium",
"version": 128
"version": 129
}
]
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
8 changes: 6 additions & 2 deletions components/dialog/dialog-fullscreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ class DialogFullscreen extends LocalizeCoreElement(AsyncContainerMixin(DialogMix
this._handleResize = this._handleResize.bind(this);
this._handleResize();
this.width = 1170;
this._titleId = getUniqueId();
}

get asyncContainerCustom() {
Expand All @@ -214,7 +215,6 @@ class DialogFullscreen extends LocalizeCoreElement(AsyncContainerMixin(DialogMix
}

render() {
this._width = Math.max(1170, this.width);
const heightOverride = {} ;
let topOverride = null;
if (this._ifrauContextInfo) {
Expand Down Expand Up @@ -251,7 +251,6 @@ class DialogFullscreen extends LocalizeCoreElement(AsyncContainerMixin(DialogMix
<div style=${styleMap(slotStyles)}><slot></slot></div>
`;

if (!this._titleId) this._titleId = getUniqueId();
Copy link
Member

Choose a reason for hiding this comment

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

I think we should just set this._titleId = getUniqueId() in the constructor. We do this already in a bunch of places, and it would avoid needing to do it in willUpdate.

const inner = html`
<div class="d2l-dialog-inner" style=${styleMap(heightOverride)}>
<div class="d2l-dialog-header">
Expand Down Expand Up @@ -286,6 +285,11 @@ class DialogFullscreen extends LocalizeCoreElement(AsyncContainerMixin(DialogMix
}
}

willUpdate(changedProperties) {
super.willUpdate(changedProperties);
if (changedProperties.has('width')) this._width = Math.max(1170, this.width);
}

_abort() {
this._close('abort');
}
Expand Down
2 changes: 1 addition & 1 deletion components/dialog/dialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ class Dialog extends LocalizeCoreElement(AsyncContainerMixin(DialogMixin(LitElem
this._criticalLabelId = getUniqueId();
this._handleResize = this._handleResize.bind(this);
this._titleId = getUniqueId();
this._textId = getUniqueId();
}

get asyncContainerCustom() {
Expand Down Expand Up @@ -158,7 +159,6 @@ class Dialog extends LocalizeCoreElement(AsyncContainerMixin(DialogMixin(LitElem
'd2l-footer-no-content': !this._hasFooterContent
};

if (!this._textId && this.describeContent) this._textId = getUniqueId();
Copy link
Member

Choose a reason for hiding this comment

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

Again, I think we should just always set this._textId in the constructor to avoid this situation. In this case, we'd be setting it sometimes when we don't need it (i.e. if this.describeContent is false), but it's cheap to generate an ID even if it doesn't get used.

const content = html`
${loading}
<div id="${ifDefined(this._textId)}" style=${styleMap(slotStyles)}><slot></slot></div>
Expand Down
14 changes: 11 additions & 3 deletions components/inputs/input-date.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,11 @@ class InputDate extends FocusMixin(LabelledMixin(SkeletonMixin(FormElementMixin(
return 'd2l-input-text';
}

/** @ignore */
get inputTextWidth() {
return `calc(${this._hiddenContentWidth} + 0.75rem + 3px)`; // text and icon width + paddingRight + border width + 1
}

/** @ignore */
get validationMessage() {
if (this.validity.rangeOverflow || this.validity.rangeUnderflow) {
Expand Down Expand Up @@ -238,9 +243,7 @@ class InputDate extends FocusMixin(LabelledMixin(SkeletonMixin(FormElementMixin(

render() {
const formattedWideDate = formatISODateInUserCalDescriptor('2323-12-23');
const inputTextWidth = `calc(${this._hiddenContentWidth} + 0.75rem + 3px)`; // text and icon width + paddingRight + border width + 1
const shortDateFormat = (this._dateTimeDescriptor.formats.dateFormats.short).toUpperCase();
this.style.maxWidth = inputTextWidth;

const clearButton = !this.required ? html`<d2l-button-subtle text="${this.localize(`${this._namespace}.clear`)}" @click="${this._handleClear}"></d2l-button-subtle>` : null;
const nowButton = this.hasNow ? html`<d2l-button-subtle text="${this.localize(`${this._namespace}.now`)}" @click="${this._handleSetToNow}"></d2l-button-subtle>` : null;
Expand Down Expand Up @@ -304,7 +307,7 @@ class InputDate extends FocusMixin(LabelledMixin(SkeletonMixin(FormElementMixin(
placeholder="${shortDateFormat}"
?required="${this.required}"
?skeleton="${this.skeleton}"
style="${styleMap({ maxWidth: inputTextWidth })}"
style="${styleMap({ maxWidth: this.inputTextWidth })}"
.value="${this._formattedValue}">
${icon}
<slot slot="inline-help" name="inline-help"></slot>
Expand Down Expand Up @@ -339,6 +342,11 @@ class InputDate extends FocusMixin(LabelledMixin(SkeletonMixin(FormElementMixin(
});
}

willUpdate(changedProperties) {
super.willUpdate(changedProperties);
if (changedProperties.has('_hiddenContentWidth')) this.style.maxWidth = this.inputTextWidth;
}

async validate() {
if (!this.shadowRoot) return;
const textInput = this.shadowRoot.querySelector('d2l-input-text');
Expand Down
7 changes: 5 additions & 2 deletions components/inputs/input-time.js
Original file line number Diff line number Diff line change
Expand Up @@ -329,11 +329,9 @@ class InputTime extends InputInlineHelpMixin(FocusMixin(LabelledMixin(SkeletonMi
`)}` : null;
const formattedWideTimeAM = formatTime(new Date(2020, 0, 1, 10, 23, 0));
const formattedWideTimePM = formatTime(new Date(2020, 0, 1, 23, 23, 0));
const inputTextWidth = `calc(${this._hiddenContentWidth} + 1.5rem + 3px)`; // text and icon width + left & right padding + border width + 1
const opened = this.opened && !this.disabled && !this.skeleton;
const dropdownIdTimezone = `${this._dropdownId}-timezone`;
const ariaDescribedByIds = `${this._dropdownId ? dropdownIdTimezone : ''} ${this._hasInlineHelp ? this._inlineHelpId : ''}`.trim();
this.style.maxWidth = inputTextWidth;

return html`
<div aria-hidden="true" class="d2l-input-time-hidden-content">
Expand Down Expand Up @@ -393,6 +391,11 @@ class InputTime extends InputInlineHelpMixin(FocusMixin(LabelledMixin(SkeletonMi
});
}

willUpdate(changedProperties) {
super.willUpdate(changedProperties);
this.style.maxWidth = `calc(${this._hiddenContentWidth} + 1.5rem + 3px)`; // text and icon width + left & right padding + border width + 1
}

getTime() {
const time = getDateFromISOTime(this.value);
return {
Expand Down
3 changes: 2 additions & 1 deletion components/skeleton/demo/skeleton-test-link.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ export class SkeletonTestLink extends SkeletonMixin(LitElement) {
'd2l-link-small': this.type === 'small',
'd2l-skeletize': true
};
const widthSkeletonSize = `d2l-skeletize-${this.width}`;
Copy link
Member

Choose a reason for hiding this comment

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

Interesting... so the new linting rules hated just referencing this.something in this case?

Copy link
Contributor

Choose a reason for hiding this comment

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

My theory is that there is an error in some regex, any reference to this on the left-hand side of an assignment will break the rule. Which makes sense to prevent assigning properties on render(), but not so much when using it as an object key.

if (this.width !== undefined) {
classes[`d2l-skeletize-${this.width}`] = true;
classes[widthSkeletonSize] = true;
}
return html`<a href="https://d2l.com" class="${classMap(classes)}">Link (${this.type})</a>`;
}
Expand Down
Binary file modified components/tooltip/test/golden/tooltip-help/chromium/clicked.png
Binary file modified components/tooltip/test/golden/tooltip-help/chromium/focused.png
Binary file modified components/tooltip/test/golden/tooltip-help/chromium/hidden.png
Binary file modified components/tooltip/test/golden/tooltip-help/chromium/hovered.png
Loading