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

Fixes for #907 #976

Merged
merged 3 commits into from
Jan 3, 2018
Merged

Fixes for #907 #976

merged 3 commits into from
Jan 3, 2018

Conversation

jimmaaay
Copy link
Contributor

@jimmaaay jimmaaay commented Jan 2, 2018

Hi there. In my PR I have included small fixes for the issue 907. Whilst doing this I noticed something small, not really worth an issue. The nc-toggle-switch button gets given the class of undefined when editing a post. I tracked this down to the file src/components/UI/Toggle getting passed classNameSwitch which in this case was not passed into it. My proposed solution for this is below.

Also really love the work that everyone put into this :)

import React from 'react'
import ReactToggled from 'react-toggled';
import c from 'classnames';

export const Toggle = ({
  active,
  onChange,
  className,
  classNameBackground,
  classNameSwitch = '',
  onFocus,
  onBlur
}) =>
  <ReactToggled on={active} onToggle={onChange}>
    {({on, getElementTogglerProps}) => (
      <span
        className={c('nc-toggle', className, { 'nc-toggle-active': on })}
        role="switch"
        aria-checked={on.toString()}
        onFocus={onFocus}
        onBlur={onBlur}
        {...getElementTogglerProps()}
      >
        <span className={`nc-toggle-background ${classNameBackground}`}/>
        <span className={`nc-toggle-switch ${classNameSwitch}`}/>
      </span>
    )}
  </ReactToggled>;

Fixes #907.

@verythorough
Copy link
Contributor

verythorough commented Jan 2, 2018

Deploy preview ready!

Built with commit 794040a

https://deploy-preview-976--netlify-cms-www.netlify.com

@verythorough
Copy link
Contributor

verythorough commented Jan 2, 2018

Deploy preview ready!

Built with commit 794040a

https://deploy-preview-976--cms-demo.netlify.com

Copy link
Contributor

@tech4him1 tech4him1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jimmaaay! I was seeing slightly different default values, can you check in your browser and change them if necessary?

@@ -22,7 +22,7 @@
& input {
background-color: #eff0f4;
border-radius: var(--borderRadius);

height: 36px; /* issue #907 */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me, this was 36.8px, can you adjust it a bit?

@@ -39,6 +39,7 @@
position: absolute;
left: 6px;
z-index: 2;
top: 9px; /* issue #907 */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me, this was 9.4px, can you adjust it a bit?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather avoid partial pixels, and I also want to avoid locking down all browsers to this solution if possible. Taking a quick look locally.

@erquhart
Copy link
Contributor

erquhart commented Jan 3, 2018

@tech4him1 updated.

height: 100%;
display: flex;
align-items: center;
pointer-events: none;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@tech4him1 tech4him1 merged commit 6fca83c into decaporg:master Jan 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI bugs in Firefox (Quantum)
4 participants