Skip to content

Animation on hover w/ Gleam Logo #2807

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

Merged
merged 45 commits into from
May 27, 2025
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
52b2af2
Animation on hover w/ Gleam Logo
Arpan-206 Apr 20, 2025
6859dfc
linting stuff
Arpan-206 Apr 20, 2025
175ee69
manually making the linter happy
Arpan-206 Apr 20, 2025
c864859
Removing debugging stuff
Arpan-206 Apr 20, 2025
c4ab10f
Working on hover too
Arpan-206 Apr 20, 2025
ea04fe0
linter wasn't feeling too happy ig
Arpan-206 Apr 20, 2025
a20cf9f
remove logs and stuff
Arpan-206 Apr 20, 2025
6cc3d5b
Add integration test
Arpan-206 Apr 20, 2025
0335cf1
linters will be the end of me
Arpan-206 Apr 20, 2025
72c9864
pretty much the last msg
Arpan-206 Apr 20, 2025
4ecb22e
quality fix?
Arpan-206 Apr 21, 2025
0ac1be3
linter
Arpan-206 Apr 21, 2025
cf62de1
tests ain't running
Arpan-206 Apr 21, 2025
1d67c55
Fixed most stuff except maybe the moseevents
Arpan-206 Apr 21, 2025
d007423
used SMs and stuff
Arpan-206 Apr 23, 2025
88fa7b8
committing if we ever need to debug animations
Arpan-206 Apr 30, 2025
257025c
Remove all the manual animation bits, fully automatic now
Arpan-206 Apr 30, 2025
e951505
Fix tests
Arpan-206 Apr 30, 2025
d798652
linter needs to be happy
Arpan-206 Apr 30, 2025
8052924
[percy] add sentry logging
Arpan-206 Apr 30, 2025
8603f7e
Changes incorporation w/o breaks
Arpan-206 Apr 30, 2025
5853d88
Linter was sad
Arpan-206 Apr 30, 2025
53f6401
It resizes dynamically
Arpan-206 May 7, 2025
711a877
Doing anything to make this work best atp
Arpan-206 May 7, 2025
733209d
Updated the animation file
Arpan-206 May 8, 2025
63cdade
move gleam logo thingy to tracks page + dynamic sizing
Arpan-206 May 8, 2025
f6bfef5
Renamed stuff to be more generic and not just gleam specific + demo
Arpan-206 May 9, 2025
a9d58af
linter worship go brrrr
Arpan-206 May 9, 2025
662128f
Add some more demos + fix tests
Arpan-206 May 9, 2025
1ea7dda
minor bunny fix
Arpan-206 May 9, 2025
2b86774
should fix the resize issues
Arpan-206 May 9, 2025
ad8dbb2
linter doin linting
Arpan-206 May 9, 2025
75d66f3
use shorthands
Arpan-206 May 9, 2025
9c91bd9
🤞
Arpan-206 May 11, 2025
bac2acc
[percy] this needed too
Arpan-206 May 11, 2025
8e9e0d0
tests are acting weird
Arpan-206 May 11, 2025
d8cfe02
Fix the non responsiveness
May 24, 2025
cfe0f6a
turns out we could use native html + tailwind for sizes
May 24, 2025
bcf6a32
SImple change to prvent horizontal stretch
May 24, 2025
df4458e
Remove not needed code
May 26, 2025
88b1138
Improve testing
May 26, 2025
dd80705
Update Animation File
May 27, 2025
092fcd5
Update to use WaitUntil
May 27, 2025
3757657
tests weren't running
May 27, 2025
6ad0f70
remove timeout
May 27, 2025
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
3 changes: 3 additions & 0 deletions app/components/gleam-logo.hbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<div {{did-insert this.handleDidInsert}} {{will-destroy this.cleanupRive}} style={{this.containerStyle}} ...attributes>
Copy link
Member

Choose a reason for hiding this comment

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

@Arpan-206 Why is the naming convention inconsistent here? did-insert uses this.handleDidInsert (correct), and will-destroy uses this.cleanupRive instead of this.handleWillDestroy?

{{! The canvas will be inserted here by Rive }}
</div>
114 changes: 114 additions & 0 deletions app/components/gleam-logo.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import { action } from '@ember/object';
import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';
import { Fit, Layout, Rive } from '@rive-app/canvas';
import * as Sentry from '@sentry/ember';

interface GleamLogoSignature {
Element: HTMLDivElement;

Args: {
height: number;
Copy link
Member

Choose a reason for hiding this comment

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

Let's try to avoid needing a number to be passed in and see if we can just have it fit the container / parent element

};

Blocks: {
default: [];
};
}

export default class GleamLogoComponent extends Component<GleamLogoSignature> {
animationInterval: number | null = null;
container: HTMLElement | null = null;
@tracked riveInstance: Rive | null = null;

Check warning on line 23 in app/components/gleam-logo.ts

View check run for this annotation

Codecov / codecov/patch

app/components/gleam-logo.ts#L23

Added line #L23 was not covered by tests
get containerStyle(): string {
// Ensure minimum size on mobile while maintaining aspect ratio
const minSize = Math.min(this.args.height, 200);
Copy link
Member

Choose a reason for hiding this comment

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

@Arpan-206 where does the "200" number come from?


return `height: ${minSize}px; width: ${minSize}px; max-width: 100%;`;
}

@action
cleanupRive() {
if (this.animationInterval) {
clearInterval(this.animationInterval);
this.animationInterval = null;
}

Check warning on line 36 in app/components/gleam-logo.ts

View check run for this annotation

Codecov / codecov/patch

app/components/gleam-logo.ts#L35-L36

Added lines #L35 - L36 were not covered by tests

if (this.riveInstance) {
this.riveInstance.stop();
this.riveInstance = null;
}
}

@action
handleDidInsert(element: HTMLDivElement) {
this.container = element;

try {
const canvas = document.createElement('canvas');

// Set initial canvas size for high quality
const baseSize = 400; // Base size for quality
Copy link
Member

Choose a reason for hiding this comment

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

@Arpan-206 let's see if we can avoid this magic number too and just have it pick a sensible value (like based on pixel density & container height)

canvas.width = baseSize;
Copy link
Member

Choose a reason for hiding this comment

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

Here are we assuming that all animations are square? Don't think we can do this, very possible to have animations that aren't square (even some of our language logos aren't square)

Copy link
Member

Choose a reason for hiding this comment

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

@Arpan-206 to ensure that we aren't doing anything specific to Gleam, let's try this with a few different Rive animations (download random ones off the internet). Extract a RiveAnimation component and add a tab for it with some examples to https://app.codecrafters.io/demo. Try to get varied kinds - with/without transparency, square vs. rectangle etc.

canvas.height = baseSize;

// Let the canvas scale naturally within its container
canvas.style.width = '100%';
canvas.style.height = '100%';
canvas.style.display = 'block';
canvas.style.maxWidth = '100%'; // Ensure it doesn't overflow on mobile

element.appendChild(canvas);

this.riveInstance = new Rive({
src: '/assets/animations/gleam_logo_animation.riv',
canvas: canvas,
stateMachines: 'State Machine 1',
Copy link
Member

Choose a reason for hiding this comment

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

It is mandatory to pass in state machine names? Will this work without it?

autoplay: true,
automaticallyHandleEvents: true,
layout: new Layout({
fit: Fit.Contain,
}),
onLoad: () => {
if (this.riveInstance) {
// Initial resize
this.riveInstance.resizeDrawingSurfaceToCanvas();

Check warning on line 77 in app/components/gleam-logo.ts

View check run for this annotation

Codecov / codecov/patch

app/components/gleam-logo.ts#L77

Added line #L77 was not covered by tests
const inputs = this.riveInstance.stateMachineInputs('State Machine 1');

Check warning on line 79 in app/components/gleam-logo.ts

View check run for this annotation

Codecov / codecov/patch

app/components/gleam-logo.ts#L79

Added line #L79 was not covered by tests
// Try to trigger hover state
if (inputs) {
Copy link
Member

Choose a reason for hiding this comment

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

Let's move most logic to the animatino file itself, and maybe name the animation / state machine onLoad

inputs.forEach((input) => {
if (input.name.toLowerCase().includes('hover')) {

Check warning on line 83 in app/components/gleam-logo.ts

View check run for this annotation

Codecov / codecov/patch

app/components/gleam-logo.ts#L83

Added line #L83 was not covered by tests
input.value = true;

Check warning on line 85 in app/components/gleam-logo.ts

View check run for this annotation

Codecov / codecov/patch

app/components/gleam-logo.ts#L85

Added line #L85 was not covered by tests
// Set timeout to trigger hover out after 1 second
setTimeout(() => {
if (this.riveInstance) {

Check warning on line 88 in app/components/gleam-logo.ts

View check run for this annotation

Codecov / codecov/patch

app/components/gleam-logo.ts#L88

Added line #L88 was not covered by tests
input.value = false;
}

Check warning on line 90 in app/components/gleam-logo.ts

View check run for this annotation

Codecov / codecov/patch

app/components/gleam-logo.ts#L90

Added line #L90 was not covered by tests
}, 1000);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Store and clean up the hover timeout.

The hover timeout is not stored or cleared when the component is destroyed, which could lead to errors if the component is removed before the timeout completes.

@tracked riveInstance: Rive | null = null;
container: HTMLElement | null = null;
animationInterval: number | null = null;
+ hoverTimeout: number | null = null;

@action
cleanupRive() {
  if (this.animationInterval) {
    clearInterval(this.animationInterval);
    this.animationInterval = null;
  }

+  if (this.hoverTimeout) {
+    clearTimeout(this.hoverTimeout);
+    this.hoverTimeout = null;
+  }

  if (this.riveInstance) {
    this.riveInstance.stop();
    this.riveInstance = null;
  }
}

// Then when setting the timeout:
- setTimeout(() => {
+ this.hoverTimeout = window.setTimeout(() => {
  if (this.riveInstance) {
    input.value = false;
  }
}, 1000);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
setTimeout(() => {
if (this.riveInstance) {
input.value = false;
}
}, 1000);
}
// In app/components/gleam-logo.ts
// Add a tracked property to hold the timeout ID
@tracked riveInstance: Rive | null = null;
container: HTMLElement | null = null;
animationInterval: number | null = null;
@tracked hoverTimeout: number | null = null;
@action
cleanupRive() {
if (this.animationInterval) {
clearInterval(this.animationInterval);
this.animationInterval = null;
}
if (this.hoverTimeout) {
clearTimeout(this.hoverTimeout);
this.hoverTimeout = null;
}
if (this.riveInstance) {
this.riveInstance.stop();
this.riveInstance = null;
}
}
// … elsewhere in your hover‐leave handler …
// Replace the bare setTimeout call with a stored timeout
this.hoverTimeout = window.setTimeout(() => {
if (this.riveInstance) {
input.value = false;
}
}, 1000);

});
}
}
},
});
} catch (error: unknown) {
console.error('Error setting up Rive:', error);
Copy link
Member

Choose a reason for hiding this comment

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

Let's report this to sentry, don't just swallow and ignore

Sentry.captureException(error, {
tags: {

Check warning on line 101 in app/components/gleam-logo.ts

View check run for this annotation

Codecov / codecov/patch

app/components/gleam-logo.ts#L100-L101

Added lines #L100 - L101 were not covered by tests
component: 'GleamLogo',
action: 'handleDidInsert',
},
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Implement fallback UI for animation failures.

The error handling reports to Sentry but doesn't provide any fallback UI when the animation fails to load. Consider adding a fallback to display a static image or message.

@tracked riveInstance: Rive | null = null;
@tracked containerSize: number = 0;
+ @tracked hasLoadError: boolean = false;

// In the catch block:
} catch (error: unknown) {
  console.error('Error setting up Rive:', error);
+  this.hasLoadError = true;
  Sentry.captureException(error, {
    tags: {
      component: 'GleamLogo',
      action: 'handleDidInsert',
    },
  });
}

Then in your template file (gleam-logo.hbs), you would add:

<div {{did-insert this.handleDidInsert}} {{will-destroy this.handleWillDestroy}} style={{this.containerStyle}}>
  {{#if this.hasLoadError}}
    <img src="/assets/images/gleam-logo.svg" alt="Gleam Logo" style="width: 100%; height: 100%;" />
  {{/if}}
</div>

Let's verify if a static fallback image exists:


🏁 Script executed:

#!/bin/bash
# Check if a static Gleam logo exists
find public/assets -type f -name "*gleam*" | grep -i '.svg\|.png\|.jpg'

# Also check if there's a generic fallback image pattern in the codebase
grep -r "hasLoadError" --include="*.hbs" app/

Length of output: 353


Fallback UI for Rive animation failures

Add a boolean flag to the component and render a static SVG when the animation fails:

• In app/components/gleam-logo.ts (around lines 100–108):

@tracked riveInstance: Rive | null = null;
@tracked containerSize: number = 0;
+ @tracked hasLoadError: boolean = false;

…  
} catch (error: unknown) {
  console.error('Error setting up Rive:', error);
+ this.hasLoadError = true;
  Sentry.captureException(error, {
    tags: {
      component: 'GleamLogo',
      action: 'handleDidInsert',
    },
  });
}

• In app/components/gleam-logo.hbs (replace /assets/images/gleam-logo.svg with an existing asset – e.g. the color version):

<div
  {{#if (not this.hasLoadError)}}
    {{did-insert this.handleDidInsert}}
    {{will-destroy this.handleWillDestroy}}
  {{/if}}
  style={{this.containerStyle}}
>
  {{#if this.hasLoadError}}
    <img
      src="/assets/images/language-logos/gleam-color.svg"
      alt="Gleam Logo"
      style="width:100%; height:100%;"
    />
  {{/if}}
</div>

Note: Available static SVGs include

  • /assets/images/language-logos/gleam-color.svg
  • /assets/images/language-logos/gleam-gray-500.svg
  • /assets/images/language-logos/gleam-teal-500.svg

Pick the variant that best matches your design.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 102-103: app/components/gleam-logo.ts#L102-L103
Added lines #L102 - L103 were not covered by tests

}
}

declare module '@glint/environment-ember-loose/registry' {
export default interface Registry {
GleamLogo: typeof GleamLogoComponent;
}
}
4 changes: 3 additions & 1 deletion app/components/language-logo.hbs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
{{#if (eq @variant "color")}}
{{#if (and (eq @language.slug "gleam") (eq @variant "color"))}}
<GleamLogo @height={{144}} ...attributes />
{{else if (eq @variant "color")}}
Copy link
Member

Choose a reason for hiding this comment

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

@Arpan-206 I mentioned this before, let's move this out to the tracks page

Screenshot 2025-04-30 at 12 29 06

<img alt={{@language.name}} src="{{@language.colorLogoUrl}}" ...attributes />
{{else if (eq @variant "gray")}}
<img alt={{@language.name}} src="{{@language.grayLogoUrl}}" ...attributes />
Expand Down
7 changes: 7 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@
},
"dependencies": {
"@rails/actioncable": "^8.0.200",
"@rive-app/canvas": "^2.27.0",
"@stripe/stripe-js": "^5.5.0",
"@tailwindcss/container-queries": "^0.1.1",
"@tailwindcss/typography": "^0.5.16",
Expand Down
Binary file not shown.
134 changes: 134 additions & 0 deletions tests/integration/components/gleam-logo-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
import { module, test } from 'qunit';
import { setupRenderingTest } from 'codecrafters-frontend/tests/helpers';
import { render, settled } from '@ember/test-helpers';
import { hbs } from 'ember-cli-htmlbars';

// Mock Rive class for testing
class MockRive {
constructor(options) {
this.canvas = options.canvas;
this._listeners = new Map();
this._onLoadCallback = null;
}

off(event, callback) {
const listeners = this._listeners.get(event);

if (listeners) {
const index = listeners.indexOf(callback);

if (index > -1) {
listeners.splice(index, 1);
}
}
}

on(event, callback) {
if (!this._listeners.has(event)) {
this._listeners.set(event, []);
}

this._listeners.get(event)?.push(callback);
}

resizeDrawingSurfaceToCanvas() {
// No-op for test
}

simulateLoad() {
if (this._onLoadCallback) {
this._onLoadCallback();
}
}

stateMachineInputs() {
return [
{
name: 'Hover',
value: false,
type: 'boolean',
},
];
}

stop() {
// No-op for test
}

triggerEvent(event, ...args) {
const listeners = this._listeners.get(event);

if (listeners) {
listeners.forEach((callback) => callback(event, ...args));
}
}
}

module('Integration | Component | gleam-logo', function (hooks) {
setupRenderingTest(hooks);

hooks.beforeEach(function () {
this.originalRive = window.Rive;
window.Rive = class MockRiveConstructor {
constructor(options) {
const instance = new MockRive(options);
this._onLoadCallback = options.onLoad;

return instance;
}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix the constructor return pattern in MockRiveConstructor

The constructor is returning a value, which is problematic in JavaScript. Constructors should initialize the object being created, not return a different object.

window.Rive = class MockRiveConstructor {
  constructor(options) {
-   const instance = new MockRive(options);
    this._onLoadCallback = options.onLoad;
-
-   return instance;
+   Object.assign(this, new MockRive(options));
  }
};

This approach ensures that this is properly initialized with the properties from MockRive while maintaining the constructor pattern's integrity.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
window.Rive = class MockRiveConstructor {
constructor(options) {
const instance = new MockRive(options);
this._onLoadCallback = options.onLoad;
return instance;
}
};
// tests/integration/components/gleam-logo-test.js
window.Rive = class MockRiveConstructor {
constructor(options) {
this._onLoadCallback = options.onLoad;
Object.assign(this, new MockRive(options));
}
};
🧰 Tools
🪛 Biome (1.9.4)

[error] 77-77: The constructor should not return a value.

The constructor is here:

Returning a value from a constructor is ignored.

(lint/correctness/noConstructorReturn)

});

hooks.afterEach(function () {
window.Rive = this.originalRive;
});

test('it renders and initializes correctly', async function (assert) {
await render(hbs`<GleamLogo @height={{200}} />`);
Copy link
Member

Choose a reason for hiding this comment

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

We're passing in height here, but the argument is called size in the component file


// Check container dimensions
const container = this.element.querySelector('div');
assert.ok(container, 'Container element exists');
assert.strictEqual(container?.style.height, '200px', 'Height is set correctly');
assert.strictEqual(container?.style.width, '200px', 'Width is set correctly');

// Check canvas dimensions and styles
const canvas = this.element.querySelector('canvas');
assert.ok(canvas, 'Canvas element exists');
assert.strictEqual(canvas?.width, 400, 'Canvas width is set to base size for quality');
assert.strictEqual(canvas?.height, 400, 'Canvas height is set to base size for quality');
assert.strictEqual(canvas?.style.width, '100%', 'Canvas width style is set correctly');
assert.strictEqual(canvas?.style.height, '100%', 'Canvas height style is set correctly');
assert.strictEqual(canvas?.style.display, 'block', 'Canvas display style is set correctly');
assert.strictEqual(canvas?.style.maxWidth, '100%', 'Canvas max-width style is set correctly');
});

test('it handles hover state correctly', async function (assert) {
await render(hbs`<GleamLogo />`);

const container = this.element.querySelector('div');
assert.ok(container, 'Container exists');

// Create and attach mock Rive instance
const mockRive = new MockRive({ canvas: container.querySelector('canvas') });
container.__riveInstance = mockRive;
assert.ok(mockRive, 'Rive instance exists');

// Simulate Rive load event
mockRive.simulateLoad();
await settled();

const inputs = mockRive.stateMachineInputs();
assert.ok(inputs, 'State machine inputs exist');

const hoverInput = inputs.find((input) => input.name.toLowerCase().includes('hover'));
assert.ok(hoverInput, 'Hover input exists');

// Verify initial hover state
assert.false(hoverInput.value, 'Hover input is initially false');

// Wait for hover out timeout
await settled();
assert.false(hoverInput.value, 'Hover input is set to false after timeout');
});
});