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

Thread post action does not have a proper loading state #227

Open
Southclaws opened this issue Oct 15, 2024 · 3 comments
Open

Thread post action does not have a proper loading state #227

Southclaws opened this issue Oct 15, 2024 · 3 comments
Labels

Comments

@Southclaws
Copy link
Owner

The useFeedMutation (or another hook, specifically for posting new threads) should properly handle loading states after clicking "Post":

  • Sonner toast via handle(..., { promiseToast })
  • Disable "Post" button
  • worth checking which hook it's already using, we probably don't need feed mutations here because you're 1. not on the feed and 2. aren't returned to the feed - so the hook itself doesn't really need to perform any optimistic mutations, the posting action doesn't need to be instant for a new thread as rolling back the state is much more complex (need to revert to editing state, load the draft, etc)
@programmingwithrp
Copy link

I like to work on this issue. can you explain more about issue ?

@Southclaws
Copy link
Owner Author

Absolutely @programmingwithrp! This can probably be done in two parts: the loading indicator on the button and the actual hook side of things. I'll detail the hook side first as it's probably slightly more complex.


Mutations in Storyden's codebase

Most API calls made from event handlers (such as form submissions, button clicks, etc) are wrapped in a function called handle which abstracts error handling so each component doesn't need to manually try-catch, deal with the error, show a toast, etc.

Currently, the compose form does not use this in its hook instead it just fires the API call directly in the doSave/doPublish handler (it's a little older than a lot of the more recent code which uses better patterns)

A good example to copy would be the Create Page Action which wraps the API call to create a page and uses the promiseToast option which implements a loading and a success toast as well as revalidation.

Revalidation is most likely not necessary to handle with creating a post, because there's nothing to revalidate. The user hits post and is immediately redirected to the new thread's page once the API call finishes so there's no need to worry about any optimistic mutation of data like with other features.

Some other bits of cleanup around here around naming: doSave -> handleSaveDraft, doPublish -> handlePublish - generally, nowadays I've leaned towards naming actual handlers handleX as opposed to onX which is used for callback props, it makes things easier to differentiate by name, but not a hard rule really.

Thank you for offering to take this issue, it's hugely appreciated especially since this is a fairly new open source project! Let me know if you have any questions! Feel free to make any changes/tidying up/best practices you see fit!

@Southclaws
Copy link
Owner Author

The button side of things may need some understanding of Panda CSS, Ark UI and the preset Park-UI. I typically copy the presets directly into Storyden's codebase (recipes/[component].ts) and use that instead of Park's CLI tool (mostly because I make minor changes to the underlying recipes anyway to fit the design system)

The Button component was copied in a long time ago, and since then it has been updated I generally avoid too much complexity around these UI components if possible, so I'd probably combine the "Add styled primitive" and "Add composition" into a single file+component (it also keeps typechecks fast as lots of composition can sometimes slow down tsc and the build)

In the Park UI code, it's not immediately clear to me why it needs a forwarded ref as the spinner just needs to be swapped in when loading is true, so I'd explore ways to simplify this before implementing. Storyden has a Spinner component already in components/ui/Spinner.

@Southclaws Southclaws moved this to Backlog in Storyden Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Backlog
Development

No branches or pull requests

2 participants