Use percentage for desktop toolbar positioning#642
Merged
Conversation
josuave
approved these changes
Dec 4, 2021
| return (absolutePos + size / 2) / total; | ||
| } | ||
|
|
||
| private double PctToAbsPos(double percentPos, double size, double total) |
Collaborator
There was a problem hiding this comment.
It doesn't have to be in this PR but we should not use shortened or abbreviated terms unless it is universally known.
Abs and Pos are pretty universal, especially in development. Percent on the other hand has variations. At this point, with modern development and computers, abbreviations in code are more a limitation imposed by either habit or laziness. Rule of thumb, anything that you name (class, method or variable) should have a name who's length is somewhat related to its scope, lifetime and complexity. Shorter lifetime/scope ~= shorter, less descriptive name
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The current absolute positioning method can be annoying, as when screen sizes change (such as RDP), the toolbar ends up in the wrong place. By using percentages, this doesn't occur.
Note that I didn't add migration for the setting. Whatever is saved from the old absolute positioning scheme will fail the bounds check and result in the default position being used. When the user moves it back to the desired location, the percentage will save. @josuave does this sound OK? Adding migration didn't seem worth it here, but open to adding it if needed.