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

Disable Post button when empty #5953

Merged
merged 3 commits into from
Oct 29, 2024
Merged

Disable Post button when empty #5953

merged 3 commits into from
Oct 29, 2024

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Oct 26, 2024

This in preparation for the threaded composer. It doesn't quite make sense to treat this as an error — we should just prevent you from posting, same as we do in other invalid cases (like too long post or missing ALT).

In the second commit, I'm also replacing ad-hoc grey disabled styling with our normal disabled button. We've already been using the disabled button for the case when you pressed post so let's use it here too.

Test Plan

Observe the initial state.

Screenshot 2024-10-26 at 20 45 16

Verify that typing makes the button enabled.

Screenshot 2024-10-26 at 20 45 20

Verify existing flows (adding image, filling alt, adding video, posting before it's uploaded) work like before.

return
}

if (isAltTextRequiredAndMissing) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

both removed conditions are a part of !canPost now

Copy link

github-actions bot commented Oct 26, 2024

Old size New size Diff
7.91 MB 7.91 MB -214 B (-0.00%)

@mozzius
Copy link
Member

mozzius commented Oct 28, 2024

Found a case where it's not disabled while empty - video errors

Screenshot 2024-10-28 at 20 34 16

@gaearon
Copy link
Collaborator Author

gaearon commented Oct 28, 2024

I think we should probably disable posting if there's an error in general? Empty or not. Clear/ack the error, then post.

@arcalinea arcalinea temporarily deployed to rm-empty-post-error - social-app PR #5953 October 28, 2024 20:43 — with Render Destroyed
@gaearon
Copy link
Collaborator Author

gaearon commented Oct 28, 2024

Pushed.

Copy link
Contributor

@haileyok haileyok left a comment

Choose a reason for hiding this comment

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

lgtm

@gaearon gaearon merged commit 3bf91eb into main Oct 29, 2024
6 checks passed
@gaearon gaearon deleted the rm-empty-post-error branch October 29, 2024 20:23
estrattonbailey added a commit that referenced this pull request Oct 31, 2024
* origin/main: (213 commits)
  add Thai Language translation support (#5879)
  fix warning on labeler profile: emoji detected but emoji not enabled (#6011)
  Added blur to search's onPressMenu (#6017)
  React compiler beta and reenable rule (#5898)
  Sort imports (#6009)
  Clarify build instructions (#6008)
  Upgrade all tiptap deps to latest (#5232)
  width full on text container (#6007)
  Bump 1.94.0 (#6006)
  Add subtle web hover to interactive rows (#5989)
  Settings revamp (#5745)
  Show almost-instant preview when opening lightbox (#6000)
  Refactor lightbox model to plain object (#5999)
  temp revert to old modal (#6005)
  Extend composer checks to all posts in a thread (#5955)
  Remove indirection when rendering composer state (#5954)
  Refactor composer state for threads (#5945)
  Disable Post button when empty (#5953)
  fix splash screen (#5997)
  Fix video quality for short videos (#5996)
  ...
rshigg pushed a commit to rshigg/social-app that referenced this pull request Nov 2, 2024
* Disable Post button when empty

* Use regular disabled button

* Disable post on video error until cleared
rshigg pushed a commit to rshigg/social-app that referenced this pull request Nov 2, 2024
* Disable Post button when empty

* Use regular disabled button

* Disable post on video error until cleared
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.

4 participants