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

Add custom system spacing #382

Merged

Conversation

brunosilvano
Copy link
Contributor

Description of Change(s)

System spacing can now be set in Preferences->General tab.

An arbitrary range from 0 to 100 was used.

I did this just to get my feet wet - let me know if anything should be changed or if this setting should actually be set somewhere else.

Appreciate any feedback =)

Fixes Issue(s)

#239

System spacing can now be set in Preferences->General tab
Copy link
Member

@cameronwhite cameronwhite left a comment

Choose a reason for hiding this comment

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

This looks good, great work!
I think putting it in the global settings seems reasonable since it's very specific to the user / display being used

I just had the one formatting note mentioned in the comments

void
ScoreArea::loadSystemSpacing(const SettingsManager &settings_manager, bool redraw)
{
auto settings = settings_manager.getReadHandle();
Copy link
Member

Choose a reason for hiding this comment

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

It looks like there's only a 2-space indent here, but it should be 4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it! Should be fixed now

@cameronwhite cameronwhite merged commit 9a51390 into powertab:master Jun 12, 2022
cameronwhite added a commit that referenced this pull request Jul 22, 2022
Any settings change (e.g. when opening a new file, the last used directory is updated in the settings) caused all open scores to be redrawn. Instead, this should only happen if the value we're interested in actually changed.
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