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 default background config #178

Merged
merged 1 commit into from Jul 9, 2018

Conversation

Projects
None yet
2 participants
@Aerijo
Copy link
Member

Aerijo commented Jun 12, 2018

Description of the Change

Adds a default background color config. Setting this makes all new panes open with that background colour (as opposed to the original hard set value of transparent).

The colours were chosen to align with those offered by the buttons in the image pane. The list order also aligns with the order of these buttons.

The default value matches the existing behaviour, so users will not notice unless they change it themselves.

It does this by setting the background colour the same way as clicking one of the colour buttons would, but as the image is being loaded.

Alternate Designs

A string input could allow custom colour choices. Enumerable is simpler and easier to understand though.

The exact wording of the config can be changed, and tests can be added if wanted.

Benefits

Users don't need to change the background colour when opening an image quite as frequently as they otherwise would.

Possible Drawbacks

None. The change will not affect users who didn't need it.

Applicable Issues

#80

Don't quite know why this was closed and labelled wontfix; seems to be some sort of bug. It's changes look a lot more complicated though, whereas this PR just sets the background colour the same way as clicking one of the colour buttons would.

@lee-dohm

This comment has been minimized.

Copy link
Member

lee-dohm commented Jun 12, 2018

I'll merge this and ship it later today. Thanks @Aerijo 🙇

@Aerijo

This comment has been minimized.

Copy link
Member Author

Aerijo commented Jul 7, 2018

@lee-dohm Not sure if you want changes, or just forgot 😄

@lee-dohm

This comment has been minimized.

Copy link
Member

lee-dohm commented Jul 9, 2018

Yep, just forgot 🙈 Thanks for the ping!

@lee-dohm lee-dohm merged commit dd135d3 into atom:master Jul 9, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@lee-dohm

This comment has been minimized.

Copy link
Member

lee-dohm commented Jul 9, 2018

And thanks for the contribution. It's updated in Atom master now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.