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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

color .setting-description alignment #1146

Merged
merged 1 commit into from Mar 26, 2020
Merged

color .setting-description alignment #1146

merged 1 commit into from Mar 26, 2020

Conversation

the-j0k3r
Copy link
Contributor

@the-j0k3r the-j0k3r commented Mar 6, 2020

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

This PR aligns the `.color .setting-description margin-top: -0.5em; pulls the description to about the same distance in e.g. checkbox settings descriptions,

Ive illustrated the description to a gif for visualization of what this PR does.

generic

Alternate Designs

Thought about actually aligning both checkbox and swatches to be middle aligned to settings titles as well as the color settings description fix in the PR Something like (can be tested by adding this to your Atom stylesheet.

.settings-view {
  input[type="checkbox"] {
    margin: 0.21em 0.75em 0 -2.25em;
  }
  .color {
    .setting-description {
      margin-top: -0.5em;
    }
  }
  input[type="color"] {
    margin: 0.313em 0 0 -5em;
  }
}

However the nature of that design choice may stray outside the intended design so I decided against submitting that for this reason.
Here it is how that would look.

generic

If that alternate design is preferred let me know.

Benefits

Accounts mainly for color settings description top alignment to label in design.

Possible Drawbacks

馃し鈥嶁檪

This proposal was tested with All built in Atom themes and across Windows, macOS, Linux.

Applicable Issues

@lee-dohm as requested on slack

@lee-dohm
Copy link
Contributor

@lee-dohm lee-dohm commented Mar 9, 2020

Thanks for submitting this @the-j0k3r!

I'll have the development team take a look 馃憤

@darangi
Copy link
Contributor

@darangi darangi commented Mar 26, 2020

Thanks for the contribution! @the-j0k3r

@darangi darangi merged commit f8b229a into atom:master Mar 26, 2020
2 checks passed
@the-j0k3r
Copy link
Contributor Author

@the-j0k3r the-j0k3r commented Mar 26, 2020

You're very welcome. =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants