refactor: remove unused DraftObject.chatId
#5684
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This should greatly simplify the code, and allow for further simplifications. This is not production-ready yet, because this removes the throttle / debounce. See the new TODO comments. However, even without the debounce, the performance doesn't seem too bad. Even if it is, the React compiler should be able to negate the effects of this commit. This also fixes a minor bug with `showSendButton`.
This is a follow-up to the previous commit, the one that lifted the draft text state from `ComposerMessageInput` to `useDraft`.
And also save the draft when switching chats, or minimizing the app. This the third and final commit in a series dedicated to refactoring and fixing up `useDraft`. The commit that removed `throttle` is actually not this one but the first one in the series, "refactor: lift composer text state up". The performance, based on Chromium profiling and looking at how long `keypress` event handler takes, seems to be pretty much unaffected compared to when we relied on `throttle` in `ComposerMessageInputProps`. It's still ~4ms. And otherwise it's possible that it will actually improve, because we send the draft to the backend much more rarely now. An indirect thing it's that the chat list has to re-render and to be refetched much less often, because `ChatlistItemChanged` fires when the draft gets saved. Closes #5252. This should also improve the situation with #3586 (but I'm not completely sure that things aren't going to get worse at first). This is also the "proper" resolution of #3733, which has previously been resolved by d1fbc7a (#4144). So this commit also somewhat reverts that one.
While working on `useDraft`, I found myself trying to implement
complex logic to manage outdated async functions,
deciding when and when not to set state.
However, `key` is just much simpler and better.
We do not rely on any kind of cross-chat state,
so having `key={chat.id}` makes sense.
The performance for switching chats appears to be about the same.
Namely, when doing - 'Share Profile' - `webxdc.sendToChat()` - bot command click - `mailto:...?body=...` click The issue was described in #5643. The problem was that, since we save the draft the the backend on debounce and not immediately, it is not enough to utilize `BackendRemote.rpc.getDraft()` to check whether a draft is present. So let's instead treat the source of truth for the draft state as being inside of `useDraft`, and overall move more draft logic to inside of `useDraft`. The problem with this, however, is that `useDraft` is not always rendered with the `accountId` and `chatId` that we need, so we have to store the "set draft request" until the correct chat has been selected and loaded.
It was introduced in 2b3cc19 (#4394), and the idea was to disable the `Ctrl + Up` shortcut (that quotes a message): #4389. However, this complicates the code unnecessarily. Loading the draft is still possible, so let's load it. With this change, and the fact that `useDraft` usage is behind React `key`, now the draft only gets reloaded when `isContactRequest` (or `inputRef`) changes.
This should not affect behavior, because `isContactRequest` is only expected to change once, and only for chats where there is no draft in the first place. This hook dependency was introduced in e7a0ce7. The issue, as usual, was that the composer input would remain disabled even after you have accepted the contact request. However, this was back when its `disabled` attribute was controlled imperatively, with `ref.setText()` or `ref.setState()` from inside of `useDraft`. Now `disabled` it's simply equal to `draftIsLoading`, which is not gonna remain `true` indefinitely. With this change, and the fact that `useDraft` usage is behind React `key`, now the draft only gets loaded once.
This basically reverts 95e808a (#5594). With `useDraft` being behind `key={(accountId, chatId)}`, this serves no purpose anymore pretty much. Well maybe except that `getDraft` won't get skipped if the component unmounted, but that's only about performance. Note that `saveAndRefetchDraft_` still suffers from race conditions.
Use `useEffectEvent` for this.
This affects the case where the draft has been modified by the user before we have managed to finish the initial `getDraft`. Previously we would always reset the draft to the "empty draft" state, even if `getDraft` returned `null` (no draft). Now we only override if there _is_ a draft stored. Although currently the composer textarea is disabled while the draft is loading, it's still possible to quote a message or add a file while the draft is loading, so this is not just a refactor.
WofWca
force-pushed
the
wofwca/useDraft-stuff-13
branch
from
d8d01e0 to
1b313a1
Compare
WofWca
force-pushed
the
wofwca/useDraft-setter-function
branch
2 times, most recently
from
04203b8 to
6b493f3
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.