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

Improve zoom scale #1944

Merged
merged 2 commits into from
Aug 8, 2018
Merged

Improve zoom scale #1944

merged 2 commits into from
Aug 8, 2018

Conversation

Zverik
Copy link
Contributor

@Zverik Zverik commented Aug 2, 2018

Fixes #1943. Zoom scale was not generated programmatically, instead I chose each value by hand, so that the scale bar is immediately readable and tends to show round values.

@Zverik Zverik mentioned this pull request Aug 2, 2018
@DylanC
Copy link
Collaborator

DylanC commented Aug 2, 2018

@ferdnyc @N3WWN - Opinions on this? I'm not fussy about zoom levels, so I don't have much to say on the matter. I guess I do prefer the shorter code in this PR, that's all I can say. 😆

@N3WWN
Copy link
Contributor

N3WWN commented Aug 2, 2018

@DylanC - Do we have all branches building AppImages at this point? Or is it just the develop and master branches?

I ask because, if it's all branches, would it make sense to put this into a new branch so we can play with these values without altering the current ones?

I will say that I do not like the current zoom values, so it's more than likely that I'll prefer these. 😜

@DylanC
Copy link
Collaborator

DylanC commented Aug 2, 2018

@N3WWN - I think its just the develop and master branches. Btw, its easy to try if you just paste the line of code over the previous. I gave it a shot just now.

I don't have much more feedback but it did feel better... if that's even considered an endorsement for it. The increments just seemed a little bit more natural and not so severe.

@N3WWN
Copy link
Contributor

N3WWN commented Aug 3, 2018

@DylanC wrote:

Btw, its easy to try if you just paste the line of code over the previous. I gave it a shot just now.

I was trying to figure out how to do this... I'm using the AppImage so have to try to intercept the reading of the file between hitting Enter to execute the AppImage and when OpenShot actually launches. The /tmp/.mount_* directory that the AppImage creates is read-only, so that's causing issues, too.

Having only spent about 15 minutes on this yesterday, that's where I'm stuck at.

@DylanC
Copy link
Collaborator

DylanC commented Aug 3, 2018

@N3WWN - Oh sorry, I mean't to say in the source code. I have it building on my machine locally. So I put this change in and gave it a whirl. Maybe @ferdnyc can try it when he has the chance. His feedback is usually quite massive and detailed. (almost the size of a little novel) 😆

@jonoomph jonoomph merged commit 7fb5f61 into OpenShot:develop Aug 8, 2018
@jonoomph
Copy link
Member

jonoomph commented Aug 8, 2018

This looks great! I just gave it a try and it works nicely! Merged.

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.

4 participants