-
-
Notifications
You must be signed in to change notification settings - Fork 203
fix: ask for draft replace confirm when needed #5666
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
WofWca
wants to merge
11
commits into
main
Choose a base branch
from
wofwca/fix-dont-ask-draft-replace-confirm-if-equal
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
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.
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.
Don't ask if nothing is actually going to get lost.
7772cf0 to
4c4ab32
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.
Don't ask if nothing is actually going to get lost.
TODO: