Open notes in new window #3110
Open notes in new window #3110
Conversation
that's not the current note in the main window
ZeroX-DG
added
needs extra review 🔎
|
@sbencoding can you resolve the conflict please? Very sorry for the wait 😢 |
|
I've resolved the conflicts, but the |
lib/note-window.js
Outdated
| }, | ||
| icon: path.resolve(basepath, '../resources/app.png') | ||
| }) | ||
| const url = path.resolve(basepath, './main.development.html') |
There was a problem hiding this comment.
We have an HTML file for production too, can you do it like in the main window:
https://github.com/BoostIO/Boostnote/blob/6ee92588b10cb9bbc5f9b43dd367ffd7439ca554/lib/main-window.js#L57-L62
browser/main/Main.js
Outdated
| noteDetailWidth: 0, | ||
| mainBodyWidth: 0 | ||
| mainBodyWidth: 0, | ||
| globRenderFlag: 1 |
There was a problem hiding this comment.
Instead of globRenderFlag can you come up with another name that express more meaning? Otherwise you can add some comments explaining what it does. But I would prefer to have a better variable name
|
|
||
| const { showFullScreen } = this.props.data | ||
|
|
||
| if (showFullScreen === undefined || showFullScreen === true) { |
There was a problem hiding this comment.
This is redundant, because === compare the type too, so you don't have to compare it to underfined.
|
|
||
| let renderFullScreenButton = true | ||
| const { showFullScreen } = this.props.data | ||
| if (showFullScreen !== undefined && showFullScreen === false) { |
There was a problem hiding this comment.
This is redundant, because === compare the type too, so you don't have to compare it to underfined.
| note: note | ||
| }) | ||
| const { showFullScreen } = this.props.data | ||
| if (showFullScreen !== undefined && showFullScreen === false) { |
There was a problem hiding this comment.
This is redundant, because === compare the type too, so you don't have to compare it to underfined.
| this.toggleLockButton = this.handleToggleLockButton.bind(this) | ||
| const { showFullScreen } = this.props.data | ||
|
|
||
| if (showFullScreen === undefined || showFullScreen === true) { |
There was a problem hiding this comment.
This is redundant, because === compare the type too, so you don't have to compare it to underfined.
| note: note | ||
| }) | ||
| const { showFullScreen } = this.props.data | ||
| if (showFullScreen !== undefined && showFullScreen === false) { |
There was a problem hiding this comment.
This is redundant, because === compare the type too, so you don't have to compare it to underfined.
|
|
||
| let renderFullScreenButton = true | ||
| const { showFullScreen } = this.props.data | ||
| if (showFullScreen !== undefined && showFullScreen === false) { |
There was a problem hiding this comment.
This is redundant, because === compare the type too, so you don't have to compare it to underfined.
browser/main/Main.js
Outdated
|
|
||
| if (this.state.globRenderFlag !== 1) { | ||
| this.props.location.search = `?key=${this.state.currentNoteKey}` | ||
| this.props.data.showFullScreen = false |
There was a problem hiding this comment.
This is strange, I don't think we can modify the props directly like this because props are read-only
https://reactjs.org/docs/components-and-props.html#props-are-read-only
|
I've implemented the requested changes 👍 |
browser/main/NoteList/index.js
Outdated
| }, | ||
| { | ||
| label: openInNewWindow, | ||
| click: this.openInNewWindow.bind(this)(note) |
There was a problem hiding this comment.
Shouldn't it be like this?
| click: this.openInNewWindow.bind(this)(note) | |
| click: this.openInNewWindow.bind(this, note) |
| const url = path.resolve( | ||
| __dirname, | ||
| process.env.NODE_ENV === 'development' | ||
| ? './main.development.html' |
There was a problem hiding this comment.
This code is not working, I think because it been called in a different place so the __dirname is changed. Can you fix the path for this?
Flexo013
added
awaiting changes 🖊️
|
I've implemented the requested changes, let me know if there's anything else I should fix or change. |
ZeroX-DG
added
approved 👍
4 participants
Description
This PR adds the ability to open and edit notes in separate, new windows.
The new windows only implement the
Detailcomponent, so there's only one main window.The
full screenbutton has been removed from the new windows, because I don't think it has purpose in that context. The editor already covers the entire window.I also had to add a new string to
i18n: Open in new window. English and Hungarian versions are correct, but for the rest of the languages I simply added the english text.Some images of the new feature:
Issue fixed
#1572 [Feature Request] Multiple notes open in their own dedicated windows.
Type of changes
Checklist:
IssueHunt Summary
Referenced issues
This pull request has been submitted to:
IssueHunt has been backed by the following sponsors. Become a sponsor