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

fix(suite): Fix firmware update progress bar #16417

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

Conversation

jvaclavik
Copy link
Contributor

@jvaclavik jvaclavik commented Jan 16, 2025

Description

  • also component and color refactoring included

Related Issue

Resolve #16215

Screenshots:

before

image

after
Screenshot 2025-01-17 at 13 57 54

Screenshot 2025-01-17 at 13 58 10 Screenshot 2025-01-17 at 13 58 54

@jvaclavik jvaclavik force-pushed the fix-firmware-update-progress-bar branch from e009972 to 4abf179 Compare January 17, 2025 12:59
@jvaclavik jvaclavik marked this pull request as ready for review January 17, 2025 13:00
@jvaclavik jvaclavik requested a review from adamhavel as a code owner January 17, 2025 13:00
)}
</Percentage>
</Wrapper>
<Box width="100%">
Copy link
Contributor Author

@jvaclavik jvaclavik Jan 17, 2025

Choose a reason for hiding this comment

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

I'm not happy about this one (it should be part of Column), but I would need to update alignItems:stretch in OnboardingStepBox which could lead many issues across the app. Not brave enough right now :D let's leave it for broader refactoring.

font-variant-numeric: tabular-nums;
height: ${spacingsPx.xl};
width: 30px;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's intended not have fixed width, because otherwise progress bar is moving according to percentage value or checkmark

height: 5px;
max-width: 100%;
width: 1%;
transform: ${({ $max, $value }) => `scaleX(${(100 / $max) * $value})`};
Copy link
Contributor Author

@jvaclavik jvaclavik Jan 17, 2025

Choose a reason for hiding this comment

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

There is a rounding bug in cases of long progressbar, so I rewrote it with width. Looks like everything is still working.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we just use Math.round?:)

@@ -80,7 +80,7 @@ export type FramePropsKeys = keyof FrameProps;
type TransientFrameProps = TransientProps<FrameProps>;

const getValueWithUnit = (value: string | number) =>
typeof value === 'string' ? value : `${value}px`;
typeof value === 'number' ? `${value}px` : value;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually this makes sense to have it this way IMHO

transform: ${({ $max, $value }) => `scaleX(${(100 / $max) * $value})`};
transform-origin: left;
transition: transform 0.5s;
width: ${({ $max, $value }) => `calc((100% / ${$max}) * ${$value})`};
Copy link
Contributor

Choose a reason for hiding this comment

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

I've changed this because it's better to use transform than to animate width:) It should be the same, do you think it bug is caused by this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I'm pretty sure it was, now it's working fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🔎 Needs review
Development

Successfully merging this pull request may close these issues.

The loading bar gets stuck at 99% even after the firmware installation is complete
3 participants