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

Refactor composer state for threads #5945

Merged
merged 2 commits into from
Oct 29, 2024
Merged

Refactor composer state for threads #5945

merged 2 commits into from
Oct 29, 2024

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Oct 25, 2024

Stacked on #5953


This prepares the composer state structure to handle multiple posts in a thread. Previously we just had the draft. Now we have a thread made of posts.

Most of the reducer logic is now in postReducer which updates PostState in response to PostAction. The postReducer is being called for the current post from the outer composerReducer.

We're not actually taking advantage of this in the UI yet. The UI is the same and it only reads the current post's state (which is always the first post because we don't have any logic to add/remove posts yet).

Also, the publishing code only looks at the first post at the moment. That will be changed later.

Test Plan

Same as usual, verify different composer features work.

@arcalinea arcalinea temporarily deployed to thread-prepare - social-app PR #5945 October 25, 2024 22:04 — with Render Destroyed
dispatch={dispatch}
post={draft}
thread={composerState.thread}
dispatch={composerDispatch}
Copy link
Collaborator Author

@gaearon gaearon Oct 25, 2024

Choose a reason for hiding this comment

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

In most cases we pass the "post" dispatch but here it needs the broader one to set post/threadgates. I'm just calling it dispatch everywhere though. Might be a bit confusing but types clarify which one it needs.

Copy link

github-actions bot commented Oct 25, 2024

Old size New size Diff
7.91 MB 7.91 MB 0 B (0.00%)

@arcalinea arcalinea temporarily deployed to thread-prepare - social-app PR #5945 October 26, 2024 01:35 — with Render Destroyed
@arcalinea arcalinea temporarily deployed to thread-prepare - social-app PR #5945 October 26, 2024 01:36 — with Render Destroyed
@gaearon gaearon changed the base branch from main to rm-empty-post-error October 26, 2024 19:52
@gaearon gaearon force-pushed the rm-empty-post-error branch from 172d464 to b29f159 Compare October 28, 2024 19:13
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, tested range of composer features without issues

Base automatically changed from rm-empty-post-error to main October 29, 2024 20:23
@arcalinea arcalinea temporarily deployed to thread-prepare - social-app PR #5945 October 29, 2024 20:27 — with Render Destroyed
@gaearon gaearon merged commit bab44a5 into main Oct 29, 2024
6 checks passed
@gaearon gaearon deleted the thread-prepare branch October 29, 2024 20:27
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
* Refactor composer state for threads

* Remove unnecessary default case

TS can see it's exhaustive.
rshigg pushed a commit to rshigg/social-app that referenced this pull request Nov 2, 2024
* Refactor composer state for threads

* Remove unnecessary default case

TS can see it's exhaustive.
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.

3 participants