Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Use "zoom to fit" as default - #81

Merged
simurai merged 4 commits into
masterfrom
sm-zoom-to-fit-as-default
Jun 24, 2017
Merged

Use "zoom to fit" as default#81
simurai merged 4 commits into
masterfrom
sm-zoom-to-fit-as-default

Conversation

@simurai

@simurai simurai commented Feb 3, 2017

Copy link
Copy Markdown
Contributor

Description of the Change

Changes the default to automatically resize an image to fit the available space.

Before After
screen shot 2017-02-03 at 4 05 16 pm screen shot 2017-02-03 at 4 04 58 pm
  • Also disables the "Zoom to fit" button once it's selected.

Alternate Designs

It could also be a config, but even then (another PR), "zoom to fit" is still the better default.

Benefits

Able to see the whole image without having to click buttons.

Possible Drawbacks

Changes the default and might upset some.

Applicable Issues

Closes #55

# Conflicts:
#	lib/image-editor-view.coffee
@simurai

simurai commented May 23, 2017

Copy link
Copy Markdown
Contributor Author

Merge conflict resolved and ready for another 👀

/cc @BinaryMuse @ungb

@ungb

ungb commented May 30, 2017

Copy link
Copy Markdown
Contributor

LGTM! This is unrelated to this PR I suppose, but when the zoom to fit button is "disabled", it still looks like a clickable button to me. This seems like the case before this pr too.

image

/cc @BinaryMuse for review

@simurai

simurai commented May 31, 2017

Copy link
Copy Markdown
Contributor Author

This is unrelated to this PR I suppose, but when the zoom to fit button is "disabled", it still looks like a clickable button to me. This seems like the case before this pr too.

The blue should mean "it's active", but yeah, to be consistent, the "Auto" button should be blue too once you start using the -+ to zoom. Made a new issue about it: #118

@iolsen

iolsen commented Jun 13, 2017

Copy link
Copy Markdown

@BinaryMuse do you have a few minutes to review this?

@simurai

simurai commented Jun 24, 2017

Copy link
Copy Markdown
Contributor Author

Ok, now that @TimvdLippe approved, we can merge it. 😄

On a serious note, it shouldn't be too risky and we have still a few weeks to test on master before it moves to Beta. 🚢

@simurai
simurai merged commit 77d2fbd into master Jun 24, 2017
@simurai
simurai deleted the sm-zoom-to-fit-as-default branch June 24, 2017 07:37
@TimvdLippe

Copy link
Copy Markdown
Contributor

It was the nicest way of saying: "oh yes please" without seeming to nag about reviewing time ;)

@BinaryMuse

Copy link
Copy Markdown

Apologies @simurai and @iolsen that this totally slipped off my radar! X_X

@aioue

aioue commented Aug 31, 2017

Copy link
Copy Markdown

Was this merged in? Running Atom 1.19.2 and image-view 0.61.2 and zoom-to-fit is not default.

@Ben3eeE

Ben3eeE commented Aug 31, 2017

Copy link
Copy Markdown
Contributor

@aioue The fix is in Atom 1.20 currently in beta.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants