-
Notifications
You must be signed in to change notification settings - Fork 25
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
Updating package-lock.json #4957
Updating package-lock.json #4957
Conversation
Thanks for the PR! 🎉 We've deployed an automatic preview for this PR - you can see your changes here:
Note The build needs to finish before your changes are deployed. |
ae9bb7c
to
6e691a2
Compare
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@@ -251,7 +250,6 @@ class DialogFullscreen extends LocalizeCoreElement(AsyncContainerMixin(DialogMix | |||
<div style=${styleMap(slotStyles)}><slot></slot></div> | |||
`; | |||
|
|||
if (!this._titleId) this._titleId = getUniqueId(); |
There was a problem hiding this comment.
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
.
@@ -286,6 +284,12 @@ class DialogFullscreen extends LocalizeCoreElement(AsyncContainerMixin(DialogMix | |||
} | |||
} | |||
|
|||
willUpdate(changedProperties) { | |||
super.willUpdate(changedProperties); | |||
this._width = Math.max(1170, this.width); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering if this should be:
if (changedProperties.has('width')) {
this._width = Math.max(1170, this.width);
}
@@ -158,7 +158,6 @@ class Dialog extends LocalizeCoreElement(AsyncContainerMixin(DialogMixin(LitElem | |||
'd2l-footer-no-content': !this._hasFooterContent | |||
}; | |||
|
|||
if (!this._textId && this.describeContent) this._textId = getUniqueId(); |
There was a problem hiding this comment.
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.
components/inputs/input-date.js
Outdated
@@ -339,6 +342,11 @@ class InputDate extends FocusMixin(LabelledMixin(SkeletonMixin(FormElementMixin( | |||
}); | |||
} | |||
|
|||
willUpdate(changedProperties) { | |||
super.willUpdate(changedProperties); | |||
this.style.maxWidth = this.inputTextWidth; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.style.maxWidth = this.inputTextWidth; | |
if (changedProperties.has('_hiddenContentWidth')) { | |
this.style.maxWidth = this.inputTextWidth; | |
} |
components/inputs/input-time.js
Outdated
const inputTextWidth = `calc(${this._hiddenContentWidth} + 1.5rem + 3px)`; // text and icon width + left & right padding + border width + 1 | ||
this.style.maxWidth = inputTextWidth; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I'd probably just merge these lines.
const inputTextWidth = `calc(${this._hiddenContentWidth} + 1.5rem + 3px)`; // text and icon width + left & right padding + border width + 1 | |
this.style.maxWidth = inputTextWidth; | |
this.style.maxWidth = `calc(${this._hiddenContentWidth} + 1.5rem + 3px)`; // text and icon width + left & right padding + border width + 1; |
@@ -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}`; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
9164c99
to
ea3b117
Compare
…com/BrightspaceUI/core into ghworkflow/package_lock_auto_update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! And then if Danny's proposal makes it through, we can revert some of this.
🎉 This PR is included in version 3.37.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Automatic update of the
package-lock.json
file.Dependency Changes