Skip to content

fix: Disable Send button when Composer is empty #35656

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

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

ishanmitra
Copy link

Description

This PR updates the logic for disabling the Send button in the message composer. Previously, the button was disabled based on both typing and isEditing, but isEditing is irrelevant to whether a message can be sent.

Changes

  • Updated the disabled condition to !canSend || !typing, ensuring the button is only enabled when text is entered.
  • Adjusted the message submission logic to prevent sending messages when the button is disabled.
  • Removed unnecessary dependency on isEditing for button state.

Why this is needed

  • The previous logic incorrectly considered isEditing, leading to cases where the button was enabled even when no text was present.
  • This fix ensures that a message can only be sent when the user has permission (canSend) and has entered text (typing).

Testing

  • Verified that the button remains disabled when the input is empty.
  • Confirmed that the button enables when text is typed.
  • Ensured that editing a message does not affect the button's disabled state.
  • Updated the disabled condition to !canSend || !typing, removing isEditing from the logic.
  • Prevented message submission when the button is disabled.
  • Ensured the button is only enabled when text is being typed, regardless of editing state.

Proposed changes (including videos or screenshots)

2cd9c59

  • Updated the disabled condition to !canSend || !typing, removing isEditing from the logic.
  • Prevented message submission when the button is disabled.
  • Ensured the button is only enabled when text is being typed, regardless of editing state.
Screen.Recording.2025-03-30.193324.mp4

Issue(s)

#35650

Further comments

35655 and 35653 are PRs that add two workflows such that a warning modal shows up if the edited text is empty.
This PR properly patches this by implementing correct condition checking without altering any chat flows.

- Updated the `disabled` condition to `!canSend || !typing`, removing `isEditing` from the logic.
- Prevented message submission when the button is disabled.
- Ensured the button is only enabled when text is being typed, regardless of editing state.
@ishanmitra ishanmitra requested a review from a team as a code owner March 30, 2025 14:16
Copy link
Contributor

dionisio-bot bot commented Mar 30, 2025

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label
  • This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

Copy link

changeset-bot bot commented Mar 30, 2025

⚠️ No Changeset found

Latest commit: f6d20f3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@juliajforesti
Copy link
Contributor

Thank you for your contribution, but actually that behavior is intended:
when you are editing a message, clear the composer and send it, it opens up the Delete message modal.

What those issues refer to is:

  • send a message
  • edit message
  • clear composer
  • hit send
  • opens delete message modal
  • cancel action

expected:
composer to still be on edit state

actual behavior:
loses edit state and composer is filled with the message it was editing

Also when you confirm the Delete action, the composer should not be filled with the previous message.

  • send a message
  • edit message
  • clear composer
  • hit send
  • opens delete message modal
  • confirm action

expected:
delete message and clear composer

actual behavior:
composer is filled with the message it was editing

Would be great if you could fix those :)

- Updated `secondary` and `info` props to depend only on `typing`, excluding `isEditing`.
- Previously, when disabled, the button remained highlighted in light blue instead of graying out, which was not visually correct. Now, it properly grays out.
@ishanmitra
Copy link
Author

ishanmitra commented Apr 2, 2025

Hi @juliajforesti !
I have made three PRs, anticipating deleting an edited message and submitting was part of the chat flow. This is the third PR, which assumes that the Send Button should be disabled in the first place.

However, I made two PRs with one key difference in how the Cancel button handles the Composer state.

The first PR 35653 does the following:

  • When you edit a message, completely remove the edited message, and the delete message modal will appear. Pressing cancel will clear the modal, the edit message state remains active, and the textarea remains empty.
  • When you press the delete button, the edit message state goes away, the text in the chatroom gets deleted, and the textarea remains empty.

The second PR 35655 does the following:

  • When you edit a message, completely remove the edited message, and the delete message modal will appear. Pressing cancel will clear the modal, the edit message state remains active, and the textarea restores the edited message.
  • When you press the delete button, the edit message state goes away, the text in the chatroom gets deleted, and the textarea remains empty. (the same as the second point for PR 35653)

The PR titles of 35653 and 35655 might be confusing. However, to put it in context, 35653 originally ensured that the expected behavior you stated is reflected in the branch I created. Meanwhile, 35655 is a branch created from 35653 that modifies a few lines of code to restore the cleared edit message. This was created as an optional choice, showcasing all the possible fixes I could come up with. I would highly recommend checking the videos for both 35653 and 35655.

I would make changes to my comments if you need any clarity as this is my first time with PRs, and I am very verbose with explanations.

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

Successfully merging this pull request may close these issues.

3 participants