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

Closed

lockee14 wants to merge 7 commits intoBoostIO:masterfrom

lockee14:Insert_Date_ISO_8601_Style

Contributor

lockee14 commented Jun 23, 2019 •

edited by issuehunt-oss bot

Loading

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

IssueHunt Summary

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


          add Date ISO 8601 format

6e45ee6

ZeroX-DG added the awaiting review ❇️ label

ZeroX-DG requested changes

View reviewed changes

browser/main/lib/ConfigManager.js Outdated

                   defaultNote: 'ALWAYS_ASK', // 'ALWAYS_ASK', 'SNIPPET_NOTE', 'MARKDOWN_NOTE'
-                  showMenuBar: false
+                  showMenuBar: false,
+                  dateFormatISO8601: false

Member

ZeroX-DG Jun 27, 2019

Shouldn’t it be editor instead of ui?

browser/main/modals/PreferencesModal/UiTab.js Outdated

                           : null
                         }
+                        <div styleName='group-checkBoxSection'>

Member

ZeroX-DG Jun 27, 2019

I think you should move this config down to editor setting

Contributor Author

lockee14 Jul 8, 2019

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

ZeroX-DG added awaiting changes 🖊️ and removed awaiting review ❇️ labels

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


          refactor: move the config down to editor setting

80a63f7

ZeroX-DG requested changes

View reviewed changes

browser/main/modals/PreferencesModal/UiTab.js Outdated

                           : null
                         }
+                        {/* <div styleName='group-checkBoxSection'>

Member

ZeroX-DG Jul 26, 2019

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

browser/main/modals/PreferencesModal/UiTab.js Outdated

                       saveTagsAlphabetically: this.refs.saveTagsAlphabetically.checked,
                       enableLiveNoteCounts: this.refs.enableLiveNoteCounts.checked,
                       showMenuBar: this.refs.showMenuBar.checked,
+                      // dateFormatISO8601: this.refs.dateFormatISO8601.checked,

Member

ZeroX-DG Jul 26, 2019

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

browser/main/lib/ConfigManager.js Outdated

                   disableDirectWrite: false,
                   defaultNote: 'ALWAYS_ASK', // 'ALWAYS_ASK', 'SNIPPET_NOTE', 'MARKDOWN_NOTE'
                   showMenuBar: false
+                  // dateFormatISO8601: false

Member

ZeroX-DG Jul 26, 2019

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

browser/components/MarkdownEditor.js Outdated

                         switchPreview={config.editor.switchPreview}
                         enableMarkdownLint={config.editor.enableMarkdownLint}
                         customMarkdownLintConfig={config.editor.customMarkdownLintConfig}
+                        // dateISO8601={config.ui.dateFormatISO8601}

Member

ZeroX-DG Jul 26, 2019

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

browser/components/MarkdownSplitEditor.js Outdated

                         switchPreview={config.editor.switchPreview}
                         enableMarkdownLint={config.editor.enableMarkdownLint}
                         customMarkdownLintConfig={config.editor.customMarkdownLintConfig}
+                        // dateISO8601={config.ui.dateFormatISO8601}

Member

ZeroX-DG Jul 26, 2019

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

browser/main/Detail/SnippetNoteDetail.js Outdated

                           enableSmartPaste={config.editor.enableSmartPaste}
                           hotkey={config.hotkey}
                           autoDetect={autoDetect}
+                          // dateISO8601={config.ui.dateFormatISO8601}

Member

ZeroX-DG Jul 26, 2019

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

Contributor Author

lockee14 Jul 29, 2019

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


          forgot to remove commented code

928e0ed

Flexo013 added awaiting review ❇️ and removed awaiting changes 🖊️ labels

Member

ZeroX-DG commented Aug 29, 2019

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

Flexo013 added awaiting changes 🖊️ and removed awaiting review ❇️ labels


          Merge branch 'master' into Insert_Date_ISO_8601_Style

f64d0b3

Contributor Author

lockee14 commented Aug 30, 2019

done

Flexo013 added awaiting review ❇️ and removed awaiting changes 🖊️ labels

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

September 12, 2019 08:14


          add a comma at the end of line 73

bdfe8c0


          add a comma at the end of line 114

b84f117


          add indentation and closing brace

2b3538d

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())
        }
},

Contributor Author

lockee14 commented Sep 11, 2019 •

edited

Loading

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

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

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

Contributor 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

ZeroX-DG mentioned this pull request

Insert Date ISO 8601 Style #3064 #3363

Closed

ZeroX-DG requested changes

View reviewed changes

Member

ZeroX-DG left a comment

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

browser/components/CodeEditor.js

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

Member

ZeroX-DG Nov 28, 2019

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())
  }
}

ZeroX-DG added awaiting changes 🖊️ and removed awaiting review ❇️ labels

Member

ZeroX-DG commented May 4, 2020

@lockee14 ping!

Member

ZeroX-DG commented May 21, 2020

@lockee14 ping 2

Member

ZeroX-DG commented Jun 15, 2020

ping3 @lockee14 if you not interested in this PR anymore I'll take over it after a week 👍

Contributor Author

lockee14 commented Jun 28, 2020

Sorry for not answering I'm too busy with other things and I don't remember what I did, it was a long time ago...

Member

ZeroX-DG commented Jun 30, 2020

That's alright, I'll take it over and finish it for you:) Unless u are interested in finish it yourself

Member

ZeroX-DG commented Jul 1, 2020

Take over here #3600

ZeroX-DG closed this

ZeroX-DG mentioned this pull request

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

Merged

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

Labels

awaiting changes 🖊️