Adds shortcuts #235

asmsuechan · 2017-01-13T12:09:09Z

Hi, I added a shortcut to focus the note as Ctrl + E. Please feedback what you feel this shortcut.

asmsuechan · 2017-01-13T13:24:38Z

And also I added a shortcut to change from Editor to Preview by ESC.

kazup01 · 2017-01-13T13:31:28Z

Greeeeeat!! Thank you for your supports!
We will check it tommorow👍

asmsuechan · 2017-01-13T13:40:03Z

browser/components/MarkdownEditor.js

@@ -142,6 +142,12 @@ class MarkdownEditor extends React.Component {
    this.renderPreview(this.props.value)
  }

+  handleKeyDown(e) {
+    if (this.state.status === 'CODE' && e.key == 'Escape') {


I think the best shortcut is Ctrl + E. In order to use it, multiple key is necessary (#227)

Oh, I often use Ctrl + E in order to move cursor.... Considering it, I'm not sure it's the best.

Is Focus notes same as toggle main? Or is the escape from editing markdown? If the escape from editing makdown, I think esc is the best.

No, it's not same. Yes, escape from editing md. By default I set ESC and I try to implement customizable key map.

Oh I see😆 Great! We'll discuss it tomorrow!

Oh, if vim keybind setting is added, ESC becomes duplicate..... Don't I have to care about it?

I think Ctrl + E is best. And it may be better to make it customizable.

Ctrl + E is best.

I agree. I'm going to try to implement multiple key handler for shortcuts.

it may be better to make it customizable.

I think so. I'll try.

OK, thaaaaank you!!

asmsuechan · 2017-01-13T15:56:48Z

And I also added a shortcut Focus Search(Ctrl + S) which toggles focus to search input. I'm not sure the name is proper or not.

asmsuechan · 2017-01-14T01:13:23Z

And I also added shortcuts to move on to the next and prior note by Ctrl + U(up) and Ctrl + J(down). Actually I want to set Ctrl + K as up by default, however it's set as a default key map on edit mode by the OS (I'm using macOS).

I'm not sure what this setting is proper or not. Please feedback 🙏

kazup01 · 2017-01-14T04:22:58Z

Hi @asmsuechan san,
Since we'll update #215 today, we'll check your PRs after this!
Thank you for your cooperation!😘

asmsuechan · 2017-01-14T10:00:14Z

browser/components/MarkdownEditor.js

@@ -8,6 +8,10 @@ class MarkdownEditor extends React.Component {
  constructor (props) {
    super(props)

+    this.hotkey = props.config.hotkey
+
+    this.keyPressed = []


I feel keyPressed should be at state however I could not write smart code by using state. I want advice about it.

I think using state better, too.
Simply, use state for keeping state of the component :)
You can add state by writing blow.

this.state = { // Other states... keyPressed: [] }

If you want to update the state, use setState method.

this.setState({ keyPressed: [a, b, c] })

If you want to write smartly, you should use the logic of Redux. But now, we can't take advantage of Redux.
So, you don't need using logic of Redux.

…menu

asmsuechan · 2017-01-14T10:32:44Z

browser/main/TopBar/index.js

@@ -244,6 +250,14 @@ class TopBar extends React.Component {
    })
  }

+  handleFocusSearch () {


Not good name

I don't think it bad name!

Actually, there is a method named handleSearchFocus()... Thus it's a little confusing 😭

Oh, I missed it.
How about renaming existing handleSearchFocus to handleOnSearchFocus?

handleOnSearchFocus is good to me, thanks your suggestion!

asmsuechan · 2017-01-14T11:32:24Z

browser/main/modals/PreferencesModal/HotkeyTab.js

@@ -138,6 +138,7 @@ class HotkeyTab extends React.Component {
                <li><code>Escape</code> (or <code>Esc</code> for short)</li>
                <li><code>VolumeUp</code>, <code>VolumeDown</code> and <code>VolumeMute</code></li>
                <li><code>MediaNextTrack</code>, <code>MediaPreviousTrack</code>, <code>MediaStop</code> and <code>MediaPlayPause</code></li>
+                <li><code>Control</code> (or <code>Ctrl</code> for short)</li>


In addition, I added this.

sota1235 · 2017-01-14T11:57:00Z

browser/main/TopBar/index.js

@@ -287,6 +301,7 @@ class TopBar extends React.Component {
                onChange={(e) => this.handleSearchChange(e)}
                placeholder='Search'
                type='text'
+                id='Search'


Why do you add id attribute?

Oops, I forgot to remove 😅

sota1235 · 2017-01-14T11:58:11Z

lib/main-menu.js

@@ -60,6 +60,13 @@ var file = {
      }
    },
    {
+      label: 'Focus Note',
+      accelerator: 'Control + E',
+      click: function () {


You can write below on ES2015+.

click() { }

I follow the code style in the same file. Should I change all of them to ES2015+ style?

Yes, I want to change coding more modern!

sota1235 · 2017-01-14T11:59:58Z

lib/main-menu.js

+      }
+    },
+    {
+      label: 'Prior Note',


I think prev is more famous. (But I'm not good at writing English. So if I was wrong, tell me please 🙇 )

I should fix it to prev because the opposite word of next seems previous.
http://www.wordhippo.com/what-is/the-opposite-of/next.html

Okay, then let's fix to previous!
Thank you for your information.

asmsuechan · 2017-01-18T03:03:56Z

I fixed them 🙏

sota1235 · 2017-01-18T14:57:14Z

lib/main-menu.js

@@ -155,7 +155,7 @@ var view = {
      }
    },
    {
-      label: 'Prior Note',
+      label: 'Privious Note',


sota1235 · 2017-01-18T15:01:35Z

browser/components/MarkdownEditor.js

    }
  }

  handleKeyUp (e) {
-    this.keyPressed[e.key] = false
+    let newKeyPressed = this.state.keyPressed
+    newKeyPressed[e.key] = false


I think using Object.prototype.assign is better way.

const keyPressed = Object.assign(this.state.keyPressed, { [e.key]: false }) this.setState({ keyPressed })

I didn't know such an useful grammar! Thanks for your advise!

sota1235 · 2017-01-18T15:02:31Z

browser/main/TopBar/index.js

@@ -250,7 +250,7 @@ class TopBar extends React.Component {
    })
  }

-  handleFocusSearch () {
+  handleOnSearchFocus () {


asmsuechan · 2017-01-20T10:17:26Z

I changed to use Object.assign and remove unnecessary lines.

…sary processing

sota1235 · 2017-01-21T05:00:06Z

lib/main-menu.js

+    {
+      label: 'Focus Search',
+      accelerator: 'Control + S',
+      click: function () {


Sorry, please write like below same as line 65 🙇

click () { }

OMG I left them 😨

refs: #235 (review)

asmsuechan · 2017-01-21T07:07:35Z

I fixed forgotten legacy style.

sota1235 · 2017-01-21T07:12:11Z

Thanks a lot!

kazup01 assigned sota1235 Jan 13, 2017

asmsuechan commented Jan 13, 2017

View reviewed changes

asmsuechan changed the title ~~Adds a shortcut which focuses the note~~ Adds shortcuts Jan 14, 2017

asmsuechan changed the title ~~Adds shortcuts~~ [WIP] Adds shortcuts Jan 14, 2017

asmsuechan changed the title ~~[WIP] Adds shortcuts~~ Adds shortcuts Jan 14, 2017

asmsuechan commented Jan 14, 2017

View reviewed changes

asmsuechan added 6 commits January 14, 2017 19:17

refs #226 Adds a shortcut which focuses the note

5bd0a44

refs #226 Adds blur shortcut on Editor

e8e05b2

refs #226 Adds focus to search shortcut to main-menu

2468c83

refs #226 Changes to toggle by Ctrl + S

b9e6a56

refs #226 Adds shortcuts which move next note and prior note to main-…

41977e8

…menu

refs #226 Adds Control to Hint of Hotkey

2ac0d93

asmsuechan commented Jan 14, 2017

View reviewed changes

refs #226 Enables to use multiple key for shortcut

cdb079d

asmsuechan commented Jan 14, 2017

View reviewed changes

sota1235 reviewed Jan 14, 2017

View reviewed changes

sota1235 assigned asmsuechan and unassigned sota1235 Jan 17, 2017

Fix pointed part by review

2b0bdbf

asmsuechan mentioned this pull request Jan 18, 2017

Multiple key mapping on Preview #227

Closed

sota1235 reviewed Jan 18, 2017

View reviewed changes

Fix typo

ec6de1b

sota1235 reviewed Jan 18, 2017

View reviewed changes

Change to use Object.assign by checking keyPressed and remove unneces…

de19c51

…sary processing

sota1235 reviewed Jan 21, 2017

View reviewed changes

Fix by review

4523743

refs: #235 (review)

sota1235 merged commit 1672d9f into BoostIO:master Jan 21, 2017

sota1235 added the Next Release label Jan 21, 2017

sota1235 mentioned this pull request Jan 21, 2017

v0.8.3 #256

Merged

sota1235 removed the Next Release label Jan 21, 2017

Uh oh!

Adds shortcuts #235

Adds shortcuts #235

Uh oh!

Conversation

asmsuechan commented Jan 13, 2017

Uh oh!

asmsuechan commented Jan 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kazup01 commented Jan 13, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kazup01 Jan 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asmsuechan Jan 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asmsuechan Jan 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asmsuechan commented Jan 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asmsuechan commented Jan 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kazup01 commented Jan 14, 2017

Uh oh!

asmsuechan Jan 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asmsuechan Jan 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asmsuechan commented Jan 13, 2017 •

edited

Loading

kazup01 Jan 13, 2017 •

edited

Loading

asmsuechan Jan 14, 2017 •

edited

Loading

asmsuechan Jan 14, 2017 •

edited

Loading

asmsuechan commented Jan 13, 2017 •

edited

Loading

asmsuechan commented Jan 14, 2017 •

edited

Loading

asmsuechan Jan 14, 2017 •

edited

Loading

asmsuechan Jan 14, 2017 •

edited

Loading

sota1235 Jan 18, 2017 •

edited

Loading