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

Add Date shortcut ISO 8601 format as an option in preference #3094

Open
wants to merge 7 commits into
base: master
from

Conversation

@lockee14
Copy link

lockee14 commented Jun 23, 2019

Description

Feature: #3064
Add an option in preference > interface to toggle the iso 8601 format for the Date shortcut using ctrl-/ and ctrl-shift-/

Issue fixed

Type of changes

  • ⚪️ Bug fix (Change that fixed an issue)
  • ⚪️ Breaking change (Change that can cause existing functionality to change)
  • ⚪️ Improvement (Change that improves the code. Maybe performance or development improvement)
  • 🔘 Feature (Change that adds new functionality)
  • ⚪️ Documentation change (Change that modifies documentation. Maybe typo fixes)

Checklist:

  • 🔘 My code follows the project code style
  • ⚪️ I have written test for my code and it has been tested
  • ⚪️ All existing tests have been passed
  • 🔘 I have attached a screenshot/video to visualize my change if possible

dateISO8601


IssueHunt Summary

IssueHunt has been backed by the following sponsors. Become a sponsor

@@ -39,7 +39,8 @@ export const DEFAULT_CONFIG = {
showCopyNotification: true,
disableDirectWrite: false,
defaultNote: 'ALWAYS_ASK', // 'ALWAYS_ASK', 'SNIPPET_NOTE', 'MARKDOWN_NOTE'
showMenuBar: false
showMenuBar: false,
dateFormatISO8601: false

This comment has been minimized.

Copy link
@ZeroX-DG

ZeroX-DG Jun 27, 2019

Member

Shouldn’t it be editor instead of ui?

@@ -293,7 +294,16 @@ class UiTab extends React.Component {
</div>
: null
}

<div styleName='group-checkBoxSection'>

This comment has been minimized.

Copy link
@ZeroX-DG

ZeroX-DG Jun 27, 2019

Member

I think you should move this config down to editor setting

This comment has been minimized.

Copy link
@lockee14

lockee14 Jul 8, 2019

Author

as you asked me I put the config in the editor setting

@ZeroX-DG ZeroX-DG changed the title Add Date shortcut ISO 8601 format as an option in preference, solve > https://github.com/BoostIO/Boostnote/issues/3064 Add Date shortcut ISO 8601 format as an option in preference Jul 1, 2019
@@ -293,7 +295,16 @@ class UiTab extends React.Component {
</div>
: null
}

{/* <div styleName='group-checkBoxSection'>

This comment has been minimized.

Copy link
@ZeroX-DG

ZeroX-DG Jul 26, 2019

Member

Please remove the code instead of just comment it out. Thank you 👍

@@ -78,6 +78,7 @@ class UiTab extends React.Component {
saveTagsAlphabetically: this.refs.saveTagsAlphabetically.checked,
enableLiveNoteCounts: this.refs.enableLiveNoteCounts.checked,
showMenuBar: this.refs.showMenuBar.checked,
// dateFormatISO8601: this.refs.dateFormatISO8601.checked,

This comment has been minimized.

Copy link
@ZeroX-DG

ZeroX-DG Jul 26, 2019

Member

Please remove the code instead of just comment it out. Thank you 👍

@@ -40,6 +40,7 @@ export const DEFAULT_CONFIG = {
disableDirectWrite: false,
defaultNote: 'ALWAYS_ASK', // 'ALWAYS_ASK', 'SNIPPET_NOTE', 'MARKDOWN_NOTE'
showMenuBar: false
// dateFormatISO8601: false

This comment has been minimized.

Copy link
@ZeroX-DG

ZeroX-DG Jul 26, 2019

Member

Please remove the code instead of just comment it out. Thank you 👍

@@ -321,6 +321,8 @@ class MarkdownEditor extends React.Component {
switchPreview={config.editor.switchPreview}
enableMarkdownLint={config.editor.enableMarkdownLint}
customMarkdownLintConfig={config.editor.customMarkdownLintConfig}
// dateISO8601={config.ui.dateFormatISO8601}

This comment has been minimized.

Copy link
@ZeroX-DG

ZeroX-DG Jul 26, 2019

Member

Please remove the code instead of just comment it out. Thank you 👍

@@ -181,6 +181,8 @@ class MarkdownSplitEditor extends React.Component {
switchPreview={config.editor.switchPreview}
enableMarkdownLint={config.editor.enableMarkdownLint}
customMarkdownLintConfig={config.editor.customMarkdownLintConfig}
// dateISO8601={config.ui.dateFormatISO8601}

This comment has been minimized.

Copy link
@ZeroX-DG

ZeroX-DG Jul 26, 2019

Member

Please remove the code instead of just comment it out. Thank you 👍

@@ -738,6 +738,8 @@ class SnippetNoteDetail extends React.Component {
enableSmartPaste={config.editor.enableSmartPaste}
hotkey={config.hotkey}
autoDetect={autoDetect}
// dateISO8601={config.ui.dateFormatISO8601}

This comment has been minimized.

Copy link
@ZeroX-DG

ZeroX-DG Jul 26, 2019

Member

Please remove the code instead of just comment it out. Thank you 👍

This comment has been minimized.

Copy link
@lockee14

lockee14 Jul 29, 2019

Author

I'm sorry I forgot to remove those before committing, it's done.

@ZeroX-DG

This comment has been minimized.

Copy link
Member

ZeroX-DG commented Aug 29, 2019

@lockee14 can you fix the conflict before I approve your code please?

@lockee14

This comment has been minimized.

Copy link
Author

lockee14 commented Aug 30, 2019

done

@Flexo013

This comment has been minimized.

Copy link
Contributor

Flexo013 commented Aug 30, 2019

@lockee14 It seems 1 check has failed. Could you look into this and fix it?

lockee14 added 3 commits Sep 11, 2019
missing indentation and brace between line 229 - 234:
before:
'Shift-Cmd-/': function (cm) {
        if (global.process.platform !== 'darwin') { return }
      [translateHotkey(hotkey.insertDateTime)]: function (cm) {
        const dateNow = new Date()
        cm.replaceSelection(dateNow.toLocaleString())
},

after:
'Shift-Cmd-/': function (cm) {
        if (global.process.platform !== 'darwin') { return }
        [translateHotkey(hotkey.insertDateTime)]: function (cm) {
          const dateNow = new Date()
          cm.replaceSelection(dateNow.toLocaleString())
        }
},
@lockee14

This comment has been minimized.

Copy link
Author

lockee14 commented Sep 11, 2019

it seems there is still an error at line 231 in CodeEditor.js
image

I thought I had fixed it by adding indentation and a brace, but there is still something missing and I don't see what

so there is something wrong here:

'Shift-Cmd-/': function (cm) {
        if (global.process.platform !== 'darwin') { return }
        [translateHotkey(hotkey.insertDateTime)]: function (cm) {
          const dateNow = new Date()
          cm.replaceSelection(dateNow.toLocaleString())
        }
      },

it should maybe be:

[translateHotkey(hotkey.insertDateTime)]: function (cm) {
        if (global.process.platform !== 'darwin') { return } // maybe also remove this?
          const dateNow = new Date()
          cm.replaceSelection(dateNow.toLocaleString())
},

I'm not sure because I don't remember this '[translateHotkey(hotkey.insertDateTime)]' and it has been a few months since I have done the Date ISO 8601 shortcut feature

@ZeroX-DG

This comment has been minimized.

Copy link
Member

ZeroX-DG commented Sep 12, 2019

Maybe it's because of this rule? This is so strange, it haven't happen before.
https://eslint.org/docs/rules/object-shorthand

I'll investigate it when I'm at home

@lockee14

This comment has been minimized.

Copy link
Author

lockee14 commented Sep 12, 2019

by looking at it a second time I think I get it, the code should be:

[translateHotkey(hotkey.insertDateTime)]: function (cm) {
          const dateNow = new Date()
          cm.replaceSelection(dateNow.toLocaleString())
},

translateHotkey seems to be here to handle compatibility issue between mac/linux/windows for the 'cmd' key thus:

'Shift-Cmd-/': function (cm) { // this is not needed anymore since translateHotkey do the work
        if (global.process.platform !== 'darwin') { return } // not needed either
        [translateHotkey(hotkey.insertDateTime)]: function (cm) { // this raise an error since it's a hash key that point to a function without being contained in a proper object variable >> var = {hashkey: something}
          const dateNow = new Date()
          cm.replaceSelection(dateNow.toLocaleString())
        }
      },

I think it should solve the issue but I'd like a second look on it since I'm not familiar with the codebase

Copy link
Member

ZeroX-DG left a comment

@lockee14 can you fix your code according to my review?

cm.replaceSelection(dateNow.toLocaleDateString())
}
},
'Cmd-/': function (cm) {

This comment has been minimized.

Copy link
@ZeroX-DG

ZeroX-DG Nov 28, 2019

Member

I think you should remove the code for Cmd-/ and Shift-Ctrl-/ and replace that with [translateHotkey(hotkey.insertDateTime)]. You code should be something like this:

[translateHotkey(hotkey.insertDateTime)]: function (cm) {
  const dateNow = new Date()
  if (self.props.dateFormatISO8601) {
       cm.replaceSelection(dateNow.toISOString())
  } else {
       cm.replaceSelection(dateNow.toLocaleString())
  }
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.