Skip to content
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

WIP: Add the rotating version of the image to the security check onboarding screen #16571

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

vojtatranta
Copy link
Contributor

@vojtatranta vojtatranta commented Jan 23, 2025

addresses: #16509

A draft PR that validates the way of how we want to properly replace those images.

ADD

  • make sure that the color is taken from the device in the "live" version
  • ✅ PRINT OUT THE pretty print of color to the name of the device
  • ✅ denser version of the animations for all cases

Description

Related Issue

Resolve

Screenshots:

@vojtatranta vojtatranta force-pushed the feat/animated-onboarding-illustrations branch from cd3c7b4 to 280c14a Compare January 23, 2025 14:35
<Wrapper>
{deviceModelInternal && (
<ImageWrapper>
<StyledImage image={`TREZOR_${deviceModelInternal}_${imageVariant}`} />
{isDeviceImageRotating ? (
// @ts-expect-error: this cannot be here as SecurityCheckLayout is used in multiple places a different API must be made for it.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jvaclavik How do we do this right? Basically, there's a "Layout" component that is used everywhere and just automatically uses the "fixed" images.

A simple boolean prop indicating that we want a rotating version of the illustration image may be enough.


return (
// @ts-expect-error: this code doesn't throw but must be deleted as it is for debugging
const searchParams = new URLSearchParams(location.search);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I marked this as a fake TS error to ensure that this must be deleted

@@ -32,6 +32,7 @@ type DeviceAnimationProps = {
isOldT2B1Packaging?: boolean;
deviceUnitColor?: number;
className?: string;
sizeVariant?: 'LARGE';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would rename it and wrote the type so that only if you pass rotate=true you could also set rotationSizeVariant to LARGE or leave it undefined.

Copy link
Contributor Author

@vojtatranta vojtatranta Jan 24, 2025

Choose a reason for hiding this comment

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

Rather, we put here imageMode: "STATIC" | "ROTATIONAL"

@@ -52,12 +53,31 @@ export const SecurityCheckLayout = ({ isFailed, children }: SecurityCheckLayoutP

const deviceModelInternal = device?.features?.internal_model;
const imageVariant = isFailed ? 'GHOST' : 'LARGE';
const isDeviceImageRotating =
deviceModelInternal &&
[DeviceModelInternal.T1B1, DeviceModelInternal.T3B1, DeviceModelInternal.T3T1, DeviceModelInternal.T2T1, DeviceModelInternal.T2B1].includes(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure if this should be here. The rotation images aren't available for all.
It shouldn't be here , it should be specified somewhere else and reused in DeviceStatus (where such list is also local)

@vojtatranta vojtatranta force-pushed the feat/animated-onboarding-illustrations branch from 280c14a to fc5ccec Compare January 24, 2025 12:45
deviceModelInternal={deviceModelInternal}
deviceUnitColor={paramColor ? Number(paramColor) : device.features?.unit_color}
height="300px"
sizeVariant={[DeviceModelInternal.T3T1, DeviceModelInternal.T2B1, DeviceModelInternal.T3B1].includes(deviceModelInternal) && [device.features?.unit_color, paramColor].map(Number).includes(4) ? 'EXTRA_LARGE' : 'LARGE'}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

DEMO: display 2x vertical video (600px) for TS3,5 color 4.

@vojtatranta vojtatranta force-pushed the feat/animated-onboarding-illustrations branch from fc5ccec to 5796b8f Compare January 27, 2025 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🎯 To do
Development

Successfully merging this pull request may close these issues.

1 participant