Skip to content
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

fix: always allow to edit the draft if it is not empty #4342

Conversation

link2xt
Copy link
Collaborator

@link2xt link2xt commented Nov 14, 2024

Otherwise if you leave the group or get removed from the group, the draft stays forever and is shown in summary,
but it is not possible to edit it or get rid of it.

This builds on top of #4340 because there is a non-working "send" button.

Otherwise if you leave the group or get removed from the group,
the draft stays forever and is shown in summary,
but it is not possible to edit it or get rid of it.
@link2xt link2xt force-pushed the link2xt/edit-nonempty-draft branch from 25669b1 to a484233 Compare November 14, 2024 21:50
@link2xt link2xt marked this pull request as ready for review November 14, 2024 21:51
@link2xt
Copy link
Collaborator Author

link2xt commented Nov 14, 2024

Composer is displayed without the send button if you cannot send but there is a draft:
draft

After removing the draft, composer disappears.

@r10s
Copy link
Member

r10s commented Nov 15, 2024

Composer is displayed without the send button if you cannot send but there is a draft

the image looks unexpected and like a bug.

we're about worsening UX here.

anyways, discussion is mainly at deltachat/deltachat-android#3428

@link2xt
Copy link
Collaborator Author

link2xt commented Nov 15, 2024

the image looks unexpected and like a bug.

Good enough for a rare case, at least it is functional. Having a draft in the chat summary that is not accessible anywhere not just looks like a bug.

@r10s
Copy link
Member

r10s commented Nov 15, 2024

exchanging one bug by the other :)

why not save some money and use up the old bug first, known at least from deltachat/signal/whatsapp 🤷‍♂️

but yeah, all points have been made already :)

@adbenitez
Copy link
Member

After removing the draft, composer disappears.

that is an unexpected UX, things appearing and disappearing without apparent reason (yes for the user it is harder to understand that it was because the draft and they would say "mmmhhh I swear there was an input bar here last time I entered this chat..." or "sometimes the send button is missing")

I agree with @r10s that this just makes UI/UX worse, it is better to discuss first the issue before jumping into implementation on every UI

@iequidoo
Copy link

For me a bad UX is that i can't send messages if i'm not a member. I'd expect that i'm not subscribed to messages from others, but i still can send and only get replies to my messages :) Basically, an app should be permissive

@link2xt
Copy link
Collaborator Author

link2xt commented Nov 16, 2024

For me a bad UX is that i can't send messages if i'm not a member. I'd expect that i'm not subscribed to messages from others, but i still can send and only get replies to my messages :) Basically, an app should be permissive

Well, group membership changes after you leave the group and there are still mailing lists that can have List-Post removed and become read-only.

@link2xt
Copy link
Collaborator Author

link2xt commented Nov 16, 2024

exchanging one bug by the other

Missing send button looks like a bug maybe, and can probably be improved with some explanation, but is not a bug, there is a reason why you can't send (not being a member or whatever).

While having something in the chat summary that is not visible anywhere in the chat and is impossible to access or even remove is a bug.

Copy link
Collaborator

@WofWca WofWca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it really is the case that core allows a draft to exist even when you can't send, then I think the draft should be accessible.
Maybe it's not the best and the most thorough way to achieve this, but I'd say this an improvement.

Comment on lines +329 to +333
!selectedChat.canSend &&
draftState.text === '' &&
draftState.file === null &&
draftState.quote === null &&
draftState.vcardContact === null
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
!selectedChat.canSend &&
draftState.text === '' &&
draftState.file === null &&
draftState.quote === null &&
draftState.vcardContact === null
!selectedChat.canSend &&
// The composer should still be accessible if there is a draft,
// even if `!canSend`.
// Firstly, to be able to remove it, secondly to be able to copy it.
// See https://github.com/deltachat/deltachat-android/issues/3428
draftState.text === '' &&
draftState.file === null &&
draftState.quote === null &&
draftState.vcardContact === null

@link2xt
Copy link
Collaborator Author

link2xt commented Nov 19, 2024

Replaced with deltachat/deltachat-core-rust#6229

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants