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

set new initial width of piano roll editor #3334

Merged
merged 1 commit into from Feb 8, 2017
Merged

Conversation

@BaraMGB
Copy link
Contributor

@BaraMGB BaraMGB commented Feb 7, 2017

fixes #3299

set the size equal to automation editor window.

@tresf
Copy link
Member

@tresf tresf commented Feb 7, 2017

The toolbar method seemed a bit more dynamic. Is there a reason why this began failing (collapsible toolbar perhaps?)

@mikobuntu
Copy link
Contributor

@mikobuntu mikobuntu commented Feb 8, 2017

@tres I'm presuming the collapsible toolbar method will still work after this PR gets implemented as it seems to only initialize a set width on opening ? , If so I think at least the user will still get these option if they decide to shrink the Piano_Roll window.
I just find that I have to stretch out the window every single time I use LMMS which can be annoying ( less clicks/dragging window size means more music being created ) .

@BaraMGB
Copy link
Contributor Author

@BaraMGB BaraMGB commented Feb 8, 2017

The toolbar method seemed a bit more dynamic. Is there a reason why this began failing (collapsible toolbar perhaps?)

@tresf yes, I guess thats the reason. The toolbar is broken into parts. I don't quite get it. Seems it uses the standard Qt Toolbar. But I don't see how to get a sizehint() from it.

@tresf
Copy link
Member

@tresf tresf commented Feb 8, 2017

@mikobuntu I think you misread my concern. My concern is why this calculated value became hard-coded, not so much about the toolbar behavior.

@BaraMGB my concern here is that as we start to write more code around high DPI displays, we need to move away from magic width and height parameters.

Since this fix is a hard requirement of 1.2, I'll merge as-is, but we need to move away from magic numbers as a project goal.

@tresf tresf merged commit ea88c90 into LMMS:master Feb 8, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.