Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI bugs in Firefox (Quantum)
4 participants