Skip to content

fix: vertically centering loading spinner in the animated class#167

Closed
bjorndonald wants to merge 1 commit intoSpiderpig86:masterfrom
bjorndonald:master
Closed

fix: vertically centering loading spinner in the animated class#167
bjorndonald wants to merge 1 commit intoSpiderpig86:masterfrom
bjorndonald:master

Conversation

@bjorndonald
Copy link
Copy Markdown
Contributor

@bjorndonald bjorndonald commented Apr 9, 2023

Description

I noticed that the loading spinner is not fully vertically centered. So I made a single line adjustment to the animation.scss. And changed the means of calculating the top property.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

Test Configuration:

  • Firmware version: macOS Monterey Version 12.6
  • Hardware: Macbook Pro
  • Toolchain: VSCode

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@Spiderpig86
Copy link
Copy Markdown
Owner

Thanks for creating a PR! I have taken a look and left a comment.

width: 1rem;
left: calc(50% - (1em / 1.25));
top: calc(50% - (1em / 1.35));
top: calc(50% - ((1rem + 2px) / 2));
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Let's remove the 2px and change it to (1rem / 2). I tried it with 0px which looks to be closer to the center and we can avoid adding more magic numbers to the framework.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Alright that works

@Spiderpig86
Copy link
Copy Markdown
Owner

Closing. Moved to #242

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