Skip to content

Add custom system spacing#382

Merged
cameronwhite merged 2 commits intopowertab:masterfrom
brunosilvano:feature/custom-system-spacing
Jun 12, 2022
Merged

Add custom system spacing#382
cameronwhite merged 2 commits intopowertab:masterfrom
brunosilvano:feature/custom-system-spacing

Conversation

@brunosilvano
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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