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

[Proposal/Discussion] How to improve indicate method #758

Open
AlbertSabate opened this issue Feb 17, 2025 · 7 comments
Open

[Proposal/Discussion] How to improve indicate method #758

AlbertSabate opened this issue Feb 17, 2025 · 7 comments
Labels
enhancement New feature or request improve DX

Comments

@AlbertSabate
Copy link
Collaborator

Describe the bug
Following the brisa example as it is, it does not work.

To Reproduce
Just follow the docs.

https://brisa.build/api-reference/components/request-context#indicate

Expected behavior
When using indicator, that label should only show when the action is pending state.

I also would like to have a way to be able to disable a button when a server action is in pending state.

API Change proposal.

Would not it be better to have something like:

const someAction = indicate(() => { action logic });

return (<>
  <button onClick={someAction}>Happy Days</button>
  {someAction.pending ? 'Action is Loading' : 'Action is loaded or not been triggered'}
</>);

The API could have the following: pending, ready, error, reset().
Problems I could think: What happen if you need to call the action multiple times?
In this case maybe we could add an id when throwing the action or proposing something like: someAction('id'). Then someAction will default to no id. And if we call someAction('...') it will contain the id. or to have it standard, maybe we could: someAction().

@AlbertSabate AlbertSabate added bug Something isn't working enhancement New feature or request improve DX labels Feb 17, 2025
@aralroca
Copy link
Collaborator

aralroca commented Feb 17, 2025

The indicator cannot condition the server rendering, because it is done on the RPC client (before the response action returns), on the web components it can be used as a signal, but on the server, it is only a CSS class that the RPC client sets during this state, where you can control it via CSS. It is implemented inspired by HTMX: https://htmx.org/attributes/hx-indicator/ It is a way to “simulate a state”, through CSS.

@aralroca
Copy link
Collaborator

@AlbertSabate Can you provide a code of what you say does not work?

This works:

const pending = indicate('some-server-action-name');
// ...
css`
 span { display: none }
 span.brisa-request { display: inline }
`
// ...
<>
  <button onClick={async () => await Bun.sleep(1000)} indicateClick={pending}>
    Run some action
  </button>
  <span indicator={pending}>Pending...</span>
</>

Video working: https://github.com/user-attachments/assets/5b3d175c-c842-4d1c-80d0-bffb5d222dbe

Note

Bun is fast, you will only see the pending state if your connection is slow. You can change the Network or put a Bun.sleep in the server action.

@AlbertSabate
Copy link
Collaborator Author

I've forgot to add the css...

However, I'm not sure I like it. So, you are able to inject a piece of css into the element, let's say with javascript. would it be possible to inject a fn or something like that for developers to control what to do with the element, instead of using brisa-request class, which may conflict with other components ?

currently I managed to solve it like:

css`
    button.brisa-request { pointer-events: none } // This disable the pointer events. But does not feel DX natural to me :/
    .brisa-request .submit-button { display: none }
    .submit-button-pending { display: none }
    .brisa-request .submit-button-pending { display: inline }
`;

@aralroca
Copy link
Collaborator

aralroca commented Feb 17, 2025

This is why indicate works also in web components, to have more control via signals 👍

https://brisa.build/api-reference/components/web-context#indicate

In the server components works very similarly to HTMX.

@AlbertSabate AlbertSabate removed the bug Something isn't working label Feb 17, 2025
@AlbertSabate
Copy link
Collaborator Author

AlbertSabate commented Feb 17, 2025

You are not replying my questions.

How do we avoid conflicts between server components?
-> Currently we will have to do .some-class-for-component .brisa-request
Would it be possible to inject a fn like a server action, where I have access to the event? Or just put some script on the fe to handle?

I would like to improve what I didn't like in the past, given that we are doing something new and hopefully widely used in the future.

I understand is similar to HTMX, however, is it good enough for Brisa?

@aralroca aralroca changed the title Indicator is not working Proposal: Improve indicator Feb 17, 2025
@aralroca aralroca changed the title Proposal: Improve indicator Proposal: Improve indicate method Feb 17, 2025
@aralroca
Copy link
Collaborator

aralroca commented Feb 17, 2025

How do we avoid conflicts between server components?

The reason that the code is like this:

const pending = indicate('some-server-action-name');
// ...
css`
 span { display: none }
 span.brisa-request { display: inline }
`
// ...
<span indicator={pending}>Pending...</span>

It's to avoid the conflict between server components. You need to specify the indicator attribute with the id of the action (is necessary to link it to the action too). So, the client RPC only adds this class to this element, not to the other server components.

Note: Maybe the useId that we are using on web components (Web Context) we need also to add it to the Request Context to improve this id generations without cost (crypto.randomUUID() has a cost), something like:

const id = useId();
const pending = indicate(`some-action-${id}`);
// ...
css`
 span { display: none }
 span.brisa-request { display: inline }
`
// ...
<span indicator={pending}>Pending...</span>

Would it be possible to inject a fn like a server action, where I have access to the event? Or just put some script on the fe to handle?

No... There is no point in adding a server action that you want to be executed before the other action is executed, to give you an idea, without an internet connection it should work, both the pending and the indicate error.

If you want to have more control, and change the render (via signals), you need to use web components (Recommended):

const pending = indicate('some-action');
// ...
<action-status indicatorName={pending}>
   <div slot="pending">Pending....</div>
   <p slot="error">Error....</p>
</action-status>

And the WC:

export default function actionStatus({ indicatorName }, { indicate }) {
   const indicator = indicate(indicatorName);

   if (indicator.value) return <slot name="pending" />;
   if (indicator.error.value) return <slot name="error" />;
   return null;
}

I understand is similar to HTMX, however, is it good enough for Brisa?

Brisa is more powerful. It mixes the power of HTMX with the power of Server Actions, plus WC.

In HTMX you can't decide whether to use JS (signals with WC) or not, there is only the option we have in the server components.

While in React, this pending/error state can only be managed with the client components, in Brisa you can use it beyond the web components, although with the limitations that no client code is created in the server components.

@AlbertSabate
Copy link
Collaborator Author

After a long discussion with @aralroca he managed to convince me this is the best way we currently have. I'm still not in love of having to use css, but it makes sense for efficiency.

Replace a class is something can be done in a one line, while inject js or do something smarter could bring bottlenecks or more complications / slowness.

I still would love to have this open for a little while in case any visionary comes with a better idea 😬

@AlbertSabate AlbertSabate changed the title Proposal: Improve indicate method [Proposal/Discussion] How to improve indicate method Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request improve DX
Projects
None yet
Development

No branches or pull requests

2 participants