Skip to content

Conversation

@asmsuechan
Copy link
Contributor

@asmsuechan asmsuechan commented Feb 11, 2017

Hi, I added a function which keeps Editor mode even though I move to another window.

29f4419dd550a0352c35930fe65ecf3f

refs: #272

@asmsuechan asmsuechan changed the title Enable lock in MarkdownEditor [WIP] Enable lock in MarkdownEditor Feb 11, 2017
@kazup01
Copy link
Member

kazup01 commented Feb 11, 2017

LGTM
Great <3

renderValue: props.value,
keyPressed: {}
keyPressed: {},
locked: false
Copy link
Contributor Author

@asmsuechan asmsuechan Feb 11, 2017

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@asmsuechan asmsuechan Feb 11, 2017

Choose a reason for hiding this comment

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

However, I'm not sure how it connects each other.

}

toggleLockButton () {
return this.state.locked ? 'fa-lock' : 'fa-unlock-alt'
Copy link
Contributor Author

@asmsuechan asmsuechan Feb 11, 2017

Choose a reason for hiding this comment

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

This does not collaborate with the actual a state of MarkdownEditor.

{(() => {
// TODO: get a state of MarkdownEditor somehow
const editorStatus='CODE'
let faClassName=`fa ${this.toggleLockButton()}`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanna get the current status PREVIEW/CODE of the MarkdownEditor.

@asmsuechan
Copy link
Contributor Author

Current status is WIP because the lock/unlock icon is shown even PREVIEW mode.

@asmsuechan
Copy link
Contributor Author

asmsuechan commented Feb 11, 2017

pic

@asmsuechan asmsuechan changed the title [WIP] Enable lock in MarkdownEditor Enable lock in MarkdownEditor Feb 11, 2017
}
this.dispatchTimer = null

this.showLockButton = () => this.handleShowLockButton()
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use Function.prototype.bind. It is more clearly which this is assigned.

this.showLockButton = this.showLockButton.bind(this);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to refactor other same style lines.

Copy link
Contributor

Choose a reason for hiding this comment

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

I know, so we should try to refactor little by little...

this.setState({ isLocked: !this.state.isLocked })
}

toggleLockButton () {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think getToggleLockButtonClassName is better.

</div>
<div styleName='info-right'>
{(() => {
let faClassName=`fa ${this.toggleLockButton()}`
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use const?

<i className={faClassName}/>
</button>
)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Writing this function before return is more readable.

const lockButtonComponent =
  <button styleName='info-right-button'
    onFocus={(e) => this.handleFocus(e)}
    onMouseDown={(e) => this.handleLockButtonMouseDown(e)}
  >
    <i className={`fa ${this.toggleLockButton()}`}/>
  </button>;

/** */

return (
  {this.state.editorStatus === 'CODE' ? lockButtonComponent : '' }
);

@asmsuechan
Copy link
Contributor Author

I fixed them.

@asmsuechan asmsuechan merged commit 6dbe3ce into BoostIO:master Mar 7, 2017
@asmsuechan asmsuechan deleted the add-lock-to-CodeEditor branch March 7, 2017 02:16
@kazup01
Copy link
Member

kazup01 commented Mar 18, 2017

I think Tooltip is needed when hover on lock icon. How about it?

@asmsuechan
Copy link
Contributor Author

@kazup01 So do I.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants