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

feat: turn methods private and fix text moving depending on number te… #178

Merged

Conversation

Harry-KNIGHT
Copy link
Contributor

When using the slider for device battery, I saw a common problem: all the text around was moving when the number is changing.

So, for fix this, I added a font modifier with monospacedDigit() in the Apple's documentation it is saying: Returns: A font that uses fixed-width numeric characters.

Before the patch

Screen.Recording.2023-11-28.at.21.41.22.mov

After the patch

Screen.Recording.2023-11-28.at.21.38.18.mov

All text is only moving when the slider value goes from 9 to 10 and 99 to 100.

I thought to add animation as the one on the Apple Watch but it is too much for a developer software.

Access control

I declared method on the view as private for being more comprehensive by the compilator and maybe win some milliseconds at build time.

If you see something to improve let me know about it.

@Harry-KNIGHT Harry-KNIGHT marked this pull request as ready for review November 28, 2023 20:48
@Harry-KNIGHT
Copy link
Contributor Author

Harry-KNIGHT commented Nov 28, 2023

@twostraws what's the convention about indentation ? I think I made something wrong on this MR so much code where touched by indentation after cmd + a cmd + i

image

I Changed indent using Spaces it fixed it.

…xt from slider

feat: change indentation to spacebar
@Harry-KNIGHT Harry-KNIGHT force-pushed the feature/text-not-moving-when-sliding branch from efcac1b to 9a37f74 Compare November 29, 2023 06:49
@twostraws
Copy link
Owner

Thank you! Regarding spacing etc, ideally the issue should be flagged up by SwiftLint, but if it isn't we should look to add a rule.

@twostraws twostraws merged commit f5b4223 into twostraws:main Dec 6, 2023
1 check passed
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