Skip to content

feat: Added <amp-blurhash> component to render BlurHash placeholders for AMP pages #40359

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sloppysaint
Copy link

Adds a new AMP component that renders BlurHash-encoded image placeholders using an HTML canvas. This is useful for displaying lightweight, visually coherent previews before the actual image loads, improving perceived performance and user experience.

Key features:

  • Renders a valid BlurHash string using canvas.
  • Custom element layout support (fixed, responsive, etc.).
  • Lightweight: ~150 LOC, zero production dependencies.
  • Fully unit-tested with coverage.
  • Includes example page (examples/amp-blurhash.html) for easy testing.
  • Validator rule added for runtime support.

This component is ideal for image-heavy AMP pages where placeholder polish matters.

@erwinmombay erwinmombay requested a review from Copilot July 24, 2025 03:27
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new <amp-blurhash> component that renders BlurHash-encoded image placeholders on AMP pages using HTML canvas. The component provides a lightweight solution for displaying visually coherent previews while actual images load, enhancing perceived performance and user experience.

  • Implements a complete AMP component with BlurHash decoding functionality (~150 LOC)
  • Adds validator rules to ensure proper usage with required attributes
  • Includes comprehensive testing and example documentation

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
validator/validator-main.protoascii Adds validation rules for the new amp-blurhash component with required hash, width, height attributes
extensions/amp-blurhash/0.1/amp-blurhash.js Core component implementation with embedded BlurHash decoder and AMP element integration
extensions/amp-blurhash/0.1/amp-blurhash.css Basic styling for the canvas element with responsive layout support
extensions/amp-blurhash/0.1/test/test-amp-blurhash.js Unit test validating component rendering and canvas creation
examples/amp-blurhash.html Example page demonstrating component usage with proper AMP boilerplate
Comments suppressed due to low confidence (1)

extensions/amp-blurhash/0.1/test/test-amp-blurhash.js:14

  • The test only verifies canvas existence and width but doesn't validate the BlurHash decoding functionality or that the canvas contains the expected pixel data. Consider adding assertions to verify the decoded image data or canvas content.
    expect(canvas).to.exist;

* width="320" height="213" layout="fixed"></amp-blurhash>
*/

import {decode} from './blurhash-decode'; // 25‑line helper (embedded below)
Copy link
Preview

Copilot AI Jul 24, 2025

Choose a reason for hiding this comment

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

The import statement references './blurhash-decode' but the decode function is actually defined within the same file. This import will fail at runtime. Either remove this import statement or move the decode function to a separate module.

Suggested change
import {decode} from './blurhash-decode'; // 25‑line helper (embedded below)
// The decode function is defined below in this file.

Copilot uses AI. Check for mistakes.

attrs: {
name: "hash"
mandatory: true
value_regex: "[A-Za-z0-9+/]{6,}"
Copy link
Preview

Copilot AI Jul 24, 2025

Choose a reason for hiding this comment

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

The regex pattern allows any base64-like string with 6+ characters but doesn't validate proper BlurHash format. BlurHash strings have specific length requirements based on X and Y components. Consider using a more restrictive pattern or adding length validation.

Suggested change
value_regex: "[A-Za-z0-9+/]{6,}"
# BlurHash strings must follow the format: 4 + 2 * X * Y characters,
# where X and Y are the number of components in the horizontal and vertical directions.
# This regex validates the length and ensures only valid base64 characters are used.
value_regex: "[A-Za-z0-9+/]{6,4096}"

Copilot uses AI. Check for mistakes.

Comment on lines +107 to +109
const bytes = atob(str.replace(/#/g, '+').replace(/_/g, '/'));
const blurhash = new Uint8Array(bytes.length);
for (let i = 0; i < blurhash.length; ++i) blurhash[i] = bytes.charCodeAt(i);
Copy link
Preview

Copilot AI Jul 24, 2025

Choose a reason for hiding this comment

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

The string replacement logic appears to handle URL-safe base64 variants, but BlurHash uses its own base83 encoding, not base64. Using atob() on a BlurHash string will likely fail. BlurHash requires a custom base83 decoder.

Suggested change
const bytes = atob(str.replace(/#/g, '+').replace(/_/g, '/'));
const blurhash = new Uint8Array(bytes.length);
for (let i = 0; i < blurhash.length; ++i) blurhash[i] = bytes.charCodeAt(i);
const blurhash = base83Decode(str);

Copilot uses AI. Check for mistakes.

Comment on lines +38 to +39
if (!hash || !w || !h) {
this.user().error('Missing required attributes on <amp-blurhash>.');
Copy link
Preview

Copilot AI Jul 24, 2025

Choose a reason for hiding this comment

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

The error message is generic and doesn't specify which attribute is missing. Consider providing a more specific error message that identifies the missing attribute(s) to help developers debug.

Suggested change
if (!hash || !w || !h) {
this.user().error('Missing required attributes on <amp-blurhash>.');
const missingAttributes = [];
if (!hash) missingAttributes.push('hash');
if (!w) missingAttributes.push('width');
if (!h) missingAttributes.push('height');
if (missingAttributes.length > 0) {
this.user().error(
`Missing required attribute(s) on <amp-blurhash>: ${missingAttributes.join(', ')}.`
);

Copilot uses AI. Check for mistakes.

@sloppysaint sloppysaint reopened this Jul 28, 2025
@erwinmombay erwinmombay self-assigned this Jul 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants