Jump to conversation
Unresolved conversations (9)
@laurent22 laurent22 Oct 30, 2021
precedentRule
Outdated
...ges/renderer/MdToHtml/rules/source_map.ts
ken1kob
@laurent22 laurent22 Oct 30, 2021
allowedLevels (camel case everywhere, as much as possible)
Outdated
...ges/renderer/MdToHtml/rules/source_map.ts
ken1kob
@laurent22 laurent22 Oct 30, 2021
allowedLevel
Outdated
...ges/renderer/MdToHtml/rules/source_map.ts
ken1kob
@laurent22 laurent22 Oct 30, 2021
===
Outdated
...es/app-desktop/gui/utils/SyncScrollMap.ts
ken1kob
@laurent22 laurent22 Oct 16, 2021
public
Outdated
...es/app-desktop/gui/utils/SyncScrollMap.ts
ken1kob laurent22
@laurent22 laurent22 Oct 16, 2021
Could you add a detailed comment on top of this code to explain what it does please.
Outdated
...es/app-desktop/gui/note-viewer/index.html
ken1kob
@laurent22 laurent22 Oct 16, 2021
onSomething Sorry to keep going with this, but I think the intend of the code is not expressed correctly. Do you need a callback or to memoize? Do you actually need any of the two or would a plain function be enough?
...Body/CodeMirror/utils/useScrollHandler.ts
@laurent22 laurent22 Oct 16, 2021
onSomething
Outdated
...Body/CodeMirror/utils/useScrollHandler.ts
@laurent22 laurent22 Oct 16, 2021
onSomething. But it looks like what you want to do is memoized some data. In that case please use `useMemo`
Outdated
...Body/CodeMirror/utils/useScrollHandler.ts
ken1kob
Resolved conversations (8)
@laurent22 laurent22 Oct 30, 2021
<s>Do you know if there's a performance impact on very large notes?</s> (sorry just saw that Caleb already checked that) Also the notes sometimes are giant blobs of HTML, when it's imported from the web clipper or Evernote. Could you check that it works with these too (in particular that it doesn't freeze)?
...es/app-desktop/gui/note-viewer/index.html
ken1kob tessus
@laurent22 laurent22 Oct 30, 2021
Check that the element exist
...es/app-desktop/gui/utils/SyncScrollMap.ts
ken1kob
@laurent22 laurent22 Oct 30, 2021
Please check that the element exist
...es/app-desktop/gui/utils/SyncScrollMap.ts
ken1kob
@laurent22 laurent22 Oct 16, 2021
Please add a detailed comment to explain what this class does.
...es/app-desktop/gui/utils/SyncScrollMap.ts
ken1kob
@laurent22 laurent22 Oct 16, 2021
onSomething
...Body/CodeMirror/utils/useScrollHandler.ts
@laurent22 laurent22 Oct 16, 2021
onSomething
...Body/CodeMirror/utils/useScrollHandler.ts
@laurent22 laurent22 Oct 16, 2021
All callbacks should be written as `onSomething`. If the onSomething notation doesn't make sense it's probably because you shouldn't make it a callback.
...Body/CodeMirror/utils/useScrollHandler.ts
@laurent22 laurent22 Oct 16, 2021
Please could you try to write this without refs? Using refs make it hard to follow the code. For example I can't tell if the timeout is properly cleared when the component unmount, and I have no idea if the state logic makes sense (i.e. stuff being updated when the state changes). I use refs too, but it has to be a last desperate attempt to get things working, and it should be documented.
...Body/CodeMirror/utils/useScrollHandler.ts
laurent22 ken1kob