zulip-desktop
zulip-desktop copied to clipboard
Left sidebar: Added ordering feature for server tabs
What's this PR do? fixes #548 Ordering feature for server tabs using drag and drop
Libraries added - SortableJS, @types/sortablejs
You have tested this PR on:
- [ ] Windows
- [x] Linux/Ubuntu
- [ ] macOS
Can you clean up this PR? Check out the Zulip commit guidelines for more details, but also please take the time to read our contributing guidelines and advice for creating reviewable pull requests.
Can you clean up this PR? Check out the Zulip commit guidelines for more details, but also please take the time to read our contributing guidelines and advice for creating reviewable pull requests.
Updated it... Let me know if any other changes are required...
Thoughts on https://github.com/zulip/zulip-desktop/pull/1059#issuecomment-812942418? Is the strategy for this PR that you're just doing a hard-reload of the app to avoid having to solve that problem?
Yeah! Server reordering is not something a user is not going to be doing very often, and given the complications of the index issues we'll be facing through other approaches, I found hard reloading to be the best way to ensure everything works properly. What are your thoughts on this?
- I’m seeing an issue where horizontal scrolling gets triggered during drag (maybe by a tooltip?).
- Repeatedly hard-reloading the app seems to mildly annoy Electron:
(node:807197) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 render-view-deleted listeners added to [WebContents]. Use emitter.setMaxListeners() to increase limit at _addListener (node:events:591:17) at WebContents.addListener (node:events:609:10) at ObjectsRegistry.registerDeleteListener (/home/anders/zulip/zulip-desktop/dist-electron/index.js:190:17) at ObjectsRegistry.add (/home/anders/zulip/zulip-desktop/dist-electron/index.js:115:12) at valueToMeta (/home/anders/zulip/zulip-desktop/dist-electron/index.js:431:40) at /home/anders/zulip/zulip-desktop/dist-electron/index.js:655:14 at IpcMainImpl.<anonymous> (/home/anders/zulip/zulip-desktop/dist-electron/index.js:585:23) at IpcMainImpl.emit (node:events:517:28) at IpcMainImpl.emit (node:domain:489:12)
Made all the changes... not sure what can be done about the warning that you are getting, Any ideas?
I think this is good to release, as the test is showing a false negative...