laurent22 / joplin

Joplin - the privacy-focused note taking app with sync capabilities for Windows, macOS, Linux, Android and iOS.
https://joplinapp.org
Other
45.84k stars 4.97k forks source link

Note list does not render properly after searching for a note #4124

Closed pluma9 closed 1 year ago

pluma9 commented 3 years ago

Environment

Joplin version: 1.4.11 Platform: Windows OS specifics: 10

Steps to reproduce

  1. Toggle the Note list to hide it
  2. Open the Command Palette and search for a notebook
  3. Jump to the notebook
  4. Toggle the Note list to show it. Now the top part of the Note list does not render until I use my mouse to scroll the list.

Describe what you expected to happen

The top part of the Note list renders properly right after the Note list shows up.

Screenshot:

Screenshot raw

Logfile

2020-11-21 23:37:00: "CommandService::execute:", "toggleSideBar", "[]"
2020-11-21 23:37:00: "CommandService::execute:", "toggleNoteList", "[]"
2020-11-21 23:37:01: "Saving settings..."
2020-11-21 23:37:01: "Settings have been saved."
2020-11-21 23:39:22: "Loading existing note", "cc62e7e650e248b5bbd753be82500755"
2020-11-21 23:39:22: "CommandService::execute:", "focusElement", "["noteBody"]"
2020-11-21 23:39:22: "CommandService::execute:", "focusElementNoteBody", "[]"
2020-11-21 23:39:22: "CodeMirror: execCommand", "{"name":"focus"}"
2020-11-21 23:39:22: App: "Refreshing notes:", "2", "287d8e351c5f496a8576096b9732c3c3"
2020-11-21 23:39:22: "Loaded note:", "{"id":"cc62e7e650e248b5bbd753be82500755","parent_id":"287d8e351c5f496a8576096b9732c3c3","title":"Bootstrap","body":"Sample","created_time":1592682267000,"updated_time":1592682267000,"is_conflict":0,"latitude":"0.00000000","longitude":"0.00000000","altitude":"0.0000","author":"","source_url":"","is_todo":0,"todo_due":0,"todo_completed":0,"source":"joplin-desktop","source_application":"net.cozic.joplin-desktop","application_data":"","order":1598717720705,"user_created_time":1592682267000,"user_updated_time":1592682267000,"encryption_cipher_text":"","encryption_applied":0,"markup_language":1,"is_shared":0,"type_":1}"
2020-11-21 23:39:22: "Saving settings..."
2020-11-21 23:39:22: "Settings have been saved."
2020-11-21 23:39:33: "CommandService::execute:", "toggleNoteList", "[]"
2020-11-21 23:39:34: "Saving settings..."
2020-11-21 23:39:34: "Settings have been saved."

Warning in development tools:

C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-dom\cjs\react-dom.development.js:11494 Warning: componentWillReceiveProps has been renamed, and is not recommended for use. See https://fb.me/react-async-component-lifecycle-hooks for details.

* Move data fetching code or side effects to componentDidUpdate.
* If you're updating state whenever props change, refactor your code to use memoization techniques or move it to static getDerivedStateFromProps. Learn more at: https://fb.me/react-derived-state
* Rename componentWillReceiveProps to UNSAFE_componentWillReceiveProps to suppress this warning in non-strict mode. In React 17.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, you can run `npx react-codemod rename-unsafe-lifecycles` in your project source folder.

Please update the following components: Connect(Dialog), Connect(HelpButtonComponent)
printWarning @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-dom\cjs\react-dom.development.js:11494
lowPriorityWarning @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-dom\cjs\react-dom.development.js:11513
ReactStrictModeWarnings.flushPendingUnsafeLifecycleWarnings @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-dom\cjs\react-dom.development.js:11680
flushRenderPhaseStrictModeWarningsInDEV @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-dom\cjs\react-dom.development.js:23134
commitRootImpl @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-dom\cjs\react-dom.development.js:22428
unstable_runWithPriority @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\scheduler\cjs\scheduler.development.js:643
runWithPriority$2 @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-dom\cjs\react-dom.development.js:11305
commitRoot @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-dom\cjs\react-dom.development.js:22414
runRootCallback @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-dom\cjs\react-dom.development.js:21554
(anonymous) @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-dom\cjs\react-dom.development.js:11353
unstable_runWithPriority @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\scheduler\cjs\scheduler.development.js:643
runWithPriority$2 @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-dom\cjs\react-dom.development.js:11305
flushSyncCallbackQueueImpl @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-dom\cjs\react-dom.development.js:11349
flushSyncCallbackQueue @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-dom\cjs\react-dom.development.js:11338
scheduleUpdateOnFiber @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-dom\cjs\react-dom.development.js:21431
enqueueSetState @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-dom\cjs\react-dom.development.js:13100
Component.setState @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react\cjs\react.development.js:471
onStateChange @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-redux\lib\components\connectAdvanced.js:222
notify @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-redux\lib\utils\Subscription.js:30
notifyNestedSubs @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-redux\lib\utils\Subscription.js:69
onStateChange @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-redux\lib\components\connectAdvanced.js:219
notify @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-redux\lib\utils\Subscription.js:30
notifyNestedSubs @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-redux\lib\utils\Subscription.js:69
onStateChange @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\react-redux\lib\components\connectAdvanced.js:219
dispatch @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\@joplin\lib\node_modules\redux\lib\createStore.js:186
(anonymous) @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\@joplin\lib\BaseApplication.js:448
(anonymous) @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\@joplin\lib\BaseApplication.js:8
__awaiter @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\@joplin\lib\BaseApplication.js:4
generalMiddleware @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\@joplin\lib\BaseApplication.js:446
(anonymous) @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\app.js:297
(anonymous) @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\app.js:8
__awaiter @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\app.js:4
generalMiddleware @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\app.js:278
(anonymous) @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\@joplin\lib\BaseApplication.js:386
dispatch @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\@joplin\lib\BaseApplication.js:561
p.instance.dispatch @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\@joplin\lib\services\PluginManager.js:42
onTrigger @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\plugins\GotoAnything.js:32
onPluginMenuItemTrigger_ @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\@joplin\lib\services\PluginManager.js:52
item.click @ C:\Users\pluma\AppData\Local\Programs\Joplin\resources\app.asar\node_modules\@joplin\lib\services\PluginManager.js:92
apply @ electron/js2c/renderer_init.js:107
(anonymous) @ electron/js2c/renderer_init.js:83
(anonymous) @ electron/js2c/renderer_init.js:83
(anonymous) @ electron/js2c/renderer_init.js:103
emit @ events.js:310
EventEmitter.emit @ domain.js:482
onMessage @ electron/js2c/renderer_init.js:91
Show 16 more frames
laurent22 commented 3 years ago

I can't replicate this on v1.4.11

pluma9 commented 3 years ago

Anyone else experiences this same issue? I can replicate this every single time I follow the above steps. Below is another screenshot of unexpectedly empty note list. After I put the mouse on the note list and scroll, that empty note list is filled with full of notes. Screenshot 2020-11-28 154654

gfrein commented 3 years ago

Same here and I too can replicate this every single time. All the notes in the note list above the one that was searched for become invisible. Nothing in the log or console, it just says CommandService::execute: toggleNoteList and then continues as usual without warnings or errors. I'm currently on 1.4.19, but it started before that.

Joplin version: 1.4.19 (AppImage) Platform: Linux OS specifics: openSUSE Tumbleweed

roman-r-m commented 3 years ago

Had a look at this issue yesterday. The root cause is that when switching to a new notebook the hightlighted item is recalculated in ItemList.updateStateItemIndexes but if the list is not visible then props.style.height is -50 so visibleItemCount and bottomItemIndex end up negative and this messes up the layout. Haven't found a good way to fix it yet.

aprvsh commented 3 years ago

@roman-r-m I am working on this. I checked and the issue still persists for the latest release on both Linux(AppImage) and Windows.

coderrsid commented 3 years ago

I am able to reproduce this on macOS too. Trying to find a solution.

image
pluma9 commented 3 years ago

@coderrsid There's an ongoing discussion about this in the forum. You may find it helpful.

prkhr-agrwl commented 3 years ago

Had a look at this issue yesterday. The root cause is that when switching to a new notebook the hightlighted item is recalculated in ItemList.updateStateItemIndexes but if the list is not visible then props.style.height is -50 so visibleItemCount and bottomItemIndex end up negative and this messes up the layout. Haven't found a good way to fix it yet.

So turns out the issue was with the scrollheight('this.scrollTop_' in ). It was getting calculated using the negative 'visibleItemCount'. So while the notelist was closed, the wrong scrollTop was in the state. But when the notelist is opened, 'props.style.height` is updated to the correct value but the scolltop is not updated in the 'updateStateItemIndexes' method. Hence, the height of the top item(which is supposed to be 0 when the scrollbar is at the top) multiplied to a large value and covered few or all of the top items depending on the number of items.

I am writing code to update the scrollTop in 'updateStateItemIndexes' method in . Please review the code when I make a pull request.

laurent22 commented 1 year ago

I've reverted the "fix" for this bug since it appears to have caused this regression: https://github.com/laurent22/joplin/issues/8237 https://github.com/laurent22/joplin/issues/8235 So it still needs to be fixed, but in a different way

laurent22 commented 1 year ago

Closing as the note list has been rewritten so there's a good chance this bug is gone