feat: implement Material 3 typography tokens and classes#120
Conversation
…s directory and update README
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces Material Design 3 Expressive indicators, adding a morphing geometric 'md-loading' component and extending 'md-progress' with wavy circular and linear tracks. It also refactors typography styles into a centralized 'md-typescale.js' module. Feedback focuses on optimizing animation performance by pausing the animation loop when inactive and caching DOM queries. Additionally, several robustness improvements are recommended, including preventing infinite loops from non-finite progress values, avoiding stack overflows from circular CSS variable references, ensuring SSR compatibility, and adding defensive null checks for canvas contexts.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| startAnimation() { | ||
| if (this.rafId) return | ||
| const canvas = this.renderRoot?.querySelector('#canvas') |
There was a problem hiding this comment.
When paused is true, the requestAnimationFrame loop continues to run continuously, clearing and redrawing the canvas on every frame. This wastes CPU/GPU resources and drains battery on mobile devices. We should prevent starting the animation loop if this.paused is true.
| startAnimation() { | |
| if (this.rafId) return | |
| const canvas = this.renderRoot?.querySelector('#canvas') | |
| startAnimation() { | |
| if (this.paused) return | |
| if (this.rafId) return | |
| const canvas = this.renderRoot?.querySelector('#canvas') |
| updated(changedProperties) { | ||
| if (changedProperties.has('size')) { | ||
| this.style.setProperty('--md-loading-size', `${this.size}px`) | ||
| const canvas = this.renderRoot.querySelector('#canvas') | ||
| if (canvas) { | ||
| this.ctx = setupCanvas(canvas, this.size) | ||
| } | ||
| } | ||
| if (changedProperties.has('color') || changedProperties.has('containerColor') || changedProperties.has('contained')) { | ||
| this.updateResolvedColors() | ||
| } | ||
| } |
There was a problem hiding this comment.
To fully optimize the paused state, we should dynamically start or stop the animation loop when the paused property changes.
updated(changedProperties) {
if (changedProperties.has('size')) {
this.style.setProperty('--md-loading-size', `${this.size}px`)
const canvas = this.renderRoot.querySelector('#canvas')
if (canvas) {
this.ctx = setupCanvas(canvas, this.size)
}
}
if (changedProperties.has('color') || changedProperties.has('containerColor') || changedProperties.has('contained')) {
this.updateResolvedColors()
}
if (changedProperties.has('paused')) {
if (this.paused) {
this.stopAnimation()
} else {
this.startAnimation()
}
}
}| _resolveColor(colorString) { | ||
| if (!colorString) return '#6750a4' | ||
| const trimmed = colorString.trim() | ||
| if (trimmed === 'currentColor') { | ||
| return window.getComputedStyle(this).color || '#6750a4' | ||
| } | ||
| if (trimmed.startsWith('var(') && trimmed.endsWith(')')) { | ||
| const content = trimmed.substring(4, trimmed.length - 1).trim() | ||
| const commaIdx = content.indexOf(',') | ||
| if (commaIdx === -1) { | ||
| const varName = content.trim() | ||
| const val = window.getComputedStyle(this).getPropertyValue(varName).trim() | ||
| return val ? this._resolveColor(val) : '#6750a4' | ||
| } else { | ||
| const varName = content.substring(0, commaIdx).trim() | ||
| const fallback = content.substring(commaIdx + 1).trim() | ||
| const val = window.getComputedStyle(this).getPropertyValue(varName).trim() | ||
| return val ? this._resolveColor(val) : this._resolveColor(fallback) | ||
| } | ||
| } | ||
| if (trimmed.startsWith('--')) { | ||
| const val = window.getComputedStyle(this).getPropertyValue(trimmed).trim() | ||
| return val ? this._resolveColor(val) : '#6750a4' | ||
| } | ||
| return trimmed | ||
| } |
There was a problem hiding this comment.
If there is a circular reference in CSS variables (e.g., --color1: var(--color2); --color2: var(--color1);), _resolveColor will recursively call itself infinitely, causing a stack overflow crash. We should limit the recursion depth to prevent this. Additionally, we should check if window is defined to support server-side rendering (SSR) environments safely.
_resolveColor(colorString, depth = 0) {
if (depth > 5) return '#6750a4'
if (!colorString) return '#6750a4'
const trimmed = colorString.trim()
const windowExists = typeof window !== 'undefined'
if (trimmed === 'currentColor') {
return (windowExists ? window.getComputedStyle(this).color : '') || '#6750a4'
}
if (trimmed.startsWith('var(') && trimmed.endsWith(')')) {
const content = trimmed.substring(4, trimmed.length - 1).trim()
const commaIdx = content.indexOf(',')
if (commaIdx === -1) {
const varName = content.trim()
const val = windowExists ? window.getComputedStyle(this).getPropertyValue(varName).trim() : ''
return val ? this._resolveColor(val, depth + 1) : '#6750a4'
} else {
const varName = content.substring(0, commaIdx).trim()
const fallback = content.substring(commaIdx + 1).trim()
const val = windowExists ? window.getComputedStyle(this).getPropertyValue(varName).trim() : ''
return val ? this._resolveColor(val, depth + 1) : this._resolveColor(fallback, depth + 1)
}
}
if (trimmed.startsWith('--')) {
const val = windowExists ? window.getComputedStyle(this).getPropertyValue(trimmed).trim() : ''
return val ? this._resolveColor(val, depth + 1) : '#6750a4'
}
return trimmed
}| function getWavyCirclePath(cx, cy, r, amplitude, frequency, phase, progress = 1.0) { | ||
| const points = [] | ||
| const steps = Math.max(10, Math.round(180 * progress)) | ||
| for (let i = 0; i <= steps; i++) { | ||
| const theta = (i / steps) * progress * 2 * Math.PI | ||
| const perturbedRadius = r + amplitude * Math.sin(frequency * theta + phase) | ||
| const x = cx + perturbedRadius * Math.cos(theta) | ||
| const y = cy + perturbedRadius * Math.sin(theta) | ||
| points.push(`${i === 0 ? 'M' : 'L'} ${x.toFixed(1)} ${y.toFixed(1)}`) | ||
| } | ||
| if (progress >= 1.0) { | ||
| points.push('Z') | ||
| } | ||
| return points.join(' ') | ||
| } |
There was a problem hiding this comment.
If progress is Infinity (which can happen if this.max is 0 and this.value > 0), steps will be Infinity, causing an infinite loop that freezes the browser tab. We must clamp progress and handle NaN / non-finite values.
| function getWavyCirclePath(cx, cy, r, amplitude, frequency, phase, progress = 1.0) { | |
| const points = [] | |
| const steps = Math.max(10, Math.round(180 * progress)) | |
| for (let i = 0; i <= steps; i++) { | |
| const theta = (i / steps) * progress * 2 * Math.PI | |
| const perturbedRadius = r + amplitude * Math.sin(frequency * theta + phase) | |
| const x = cx + perturbedRadius * Math.cos(theta) | |
| const y = cy + perturbedRadius * Math.sin(theta) | |
| points.push(`${i === 0 ? 'M' : 'L'} ${x.toFixed(1)} ${y.toFixed(1)}`) | |
| } | |
| if (progress >= 1.0) { | |
| points.push('Z') | |
| } | |
| return points.join(' ') | |
| } | |
| function getWavyCirclePath(cx, cy, r, amplitude, frequency, phase, progress = 1.0) { | |
| if (isNaN(progress) || !isFinite(progress)) { | |
| progress = 0 | |
| } | |
| progress = Math.max(0, Math.min(1, progress)) | |
| const points = [] | |
| const steps = Math.max(10, Math.round(180 * progress)) | |
| for (let i = 0; i <= steps; i++) { | |
| const theta = (i / steps) * progress * 2 * Math.PI | |
| const perturbedRadius = r + amplitude * Math.sin(frequency * theta + phase) | |
| const x = cx + perturbedRadius * Math.cos(theta) | |
| const y = cy + perturbedRadius * Math.sin(theta) | |
| points.push(`${i === 0 ? 'M' : 'L'} ${x.toFixed(1)} ${y.toFixed(1)}`) | |
| } | |
| if (progress >= 1.0) { | |
| points.push('Z') | |
| } | |
| return points.join(' ') | |
| } |
| function setupCanvas(canvas, cssSize) { | ||
| const dpr = typeof window !== "undefined" ? window.devicePixelRatio || 1 : 1 | ||
| const px = Math.round(cssSize * dpr) | ||
| canvas.width = px | ||
| canvas.height = px | ||
| canvas.style.width = `${cssSize}px` | ||
| canvas.style.height = `${cssSize}px` | ||
| const ctx = canvas.getContext("2d") | ||
| ctx.scale(dpr, dpr) | ||
| return ctx | ||
| } |
There was a problem hiding this comment.
If canvas.getContext("2d") returns null (which can happen if the browser is out of memory or canvas is disabled), calling ctx.scale will throw a TypeError and crash the component. We should add a defensive null check.
| function setupCanvas(canvas, cssSize) { | |
| const dpr = typeof window !== "undefined" ? window.devicePixelRatio || 1 : 1 | |
| const px = Math.round(cssSize * dpr) | |
| canvas.width = px | |
| canvas.height = px | |
| canvas.style.width = `${cssSize}px` | |
| canvas.style.height = `${cssSize}px` | |
| const ctx = canvas.getContext("2d") | |
| ctx.scale(dpr, dpr) | |
| return ctx | |
| } | |
| function setupCanvas(canvas, cssSize) { | |
| const dpr = typeof window !== "undefined" ? window.devicePixelRatio || 1 : 1 | |
| const px = Math.round(cssSize * dpr) | |
| canvas.width = px | |
| canvas.height = px | |
| canvas.style.width = `${cssSize}px` | |
| canvas.style.height = `${cssSize}px` | |
| const ctx = canvas.getContext("2d") | |
| if (ctx) { | |
| ctx.scale(dpr, dpr) | |
| } | |
| return ctx | |
| } |
| update(ts) { | ||
| if (this.paused) { | ||
| this.lastTs = ts | ||
| return | ||
| } | ||
| if (this.lastTs === 0) this.lastTs = ts | ||
| const rawDt = Math.min((ts - this.lastTs) / 1e3, 0.1) | ||
| const dt = rawDt * this.speed | ||
| this.lastTs = ts | ||
| if (dt <= 0) return |
There was a problem hiding this comment.
If this.speed is set to NaN or a non-finite value, dt will be NaN or non-finite, which propagates and crashes the animation loop. We should guard against this.
| update(ts) { | |
| if (this.paused) { | |
| this.lastTs = ts | |
| return | |
| } | |
| if (this.lastTs === 0) this.lastTs = ts | |
| const rawDt = Math.min((ts - this.lastTs) / 1e3, 0.1) | |
| const dt = rawDt * this.speed | |
| this.lastTs = ts | |
| if (dt <= 0) return | |
| update(ts) { | |
| if (this.paused) { | |
| this.lastTs = ts | |
| return | |
| } | |
| if (this.lastTs === 0) this.lastTs = ts | |
| const rawDt = Math.min((ts - this.lastTs) / 1e3, 0.1) | |
| const dt = rawDt * this.speed | |
| this.lastTs = ts | |
| if (isNaN(dt) || !isFinite(dt) || dt <= 0) return |
| startWavyAnimation() { | ||
| if (this.wavyAnimationActive) return | ||
| this.wavyAnimationActive = true | ||
| this.wavyPhase = 0 | ||
| const loop = (ts) => { | ||
| if (!this.wavyAnimationActive) return | ||
| this.wavyPhase += 0.08 | ||
| const path = this.renderRoot.querySelector('.active-track.wavy-circle') | ||
| if (path) { | ||
| const isIndeterminateWavy = this.indeterminate && this.shape === 'wavy' | ||
| const progress = isIndeterminateWavy ? 0.25 : (this.value / this.max) | ||
| path.setAttribute('d', getWavyCirclePath(2400, 2400, 2160, 120, 12, this.wavyPhase, progress)) | ||
| } | ||
| this.wavyRafId = requestAnimationFrame(loop) | ||
| } | ||
| this.wavyRafId = requestAnimationFrame(loop) | ||
| } |
There was a problem hiding this comment.
Querying the DOM on every frame using querySelector is a known performance anti-pattern in Web Components / Lit. We should cache the path element reference.
startWavyAnimation() {
if (this.wavyAnimationActive) return
this.wavyAnimationActive = true
this.wavyPhase = 0
const loop = (ts) => {
if (!this.wavyAnimationActive) return
this.wavyPhase += 0.08
if (!this.wavyCirclePath) {
this.wavyCirclePath = this.renderRoot.querySelector('.active-track.wavy-circle')
}
const path = this.wavyCirclePath
if (path) {
const isIndeterminateWavy = this.indeterminate && this.shape === 'wavy'
const progress = isIndeterminateWavy ? 0.25 : (this.value / this.max)
path.setAttribute('d', getWavyCirclePath(2400, 2400, 2160, 120, 12, this.wavyPhase, progress))
}
this.wavyRafId = requestAnimationFrame(loop)
}
this.wavyRafId = requestAnimationFrame(loop)
}| updated(changedProperties) { | ||
| if (changedProperties.has('shape') || changedProperties.has('type')) { | ||
| this.updateWavyAnimationState() | ||
| } | ||
| } |
There was a problem hiding this comment.
We should clear the cached path reference when shape or type changes to ensure the correct element is queried next time.
| updated(changedProperties) { | |
| if (changedProperties.has('shape') || changedProperties.has('type')) { | |
| this.updateWavyAnimationState() | |
| } | |
| } | |
| updated(changedProperties) { | |
| if (changedProperties.has('shape') || changedProperties.has('type')) { | |
| this.wavyCirclePath = null | |
| this.updateWavyAnimationState() | |
| } | |
| } |
1 participant
This PR adds the M3 type scale tokens, classes, and helper styles under typography/md-typescale.js and updates md-typescale-styles.js for backward compatibility.