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

[LiveComponent] Not write url query param when empty/default #2141

Open
Nayte91 opened this issue Sep 9, 2024 · 4 comments
Open

[LiveComponent] Not write url query param when empty/default #2141

Nayte91 opened this issue Sep 9, 2024 · 4 comments

Comments

@Nayte91
Copy link

Nayte91 commented Sep 9, 2024

Hello,

Part of the discussion about faceted search/result combo (challenge point 3), I'm playing with live components and the feature to change an url when a LiveProp changes; I was wondering if there is actually a setting to remove the given param when its empty.

I explain a bit:
I have a "name" field wired to a live component with url: true. Assuming I write inside, my url will be http://localhost/index?name=foo. The question is if I reset the field, the URL will be http://localhost/index?name= but is there a way to enforce url to something like http://localhost/index? It's cleaner, yes, but I think it's not trivia if you have a lot of params, because of a faceted search or other: http://localhost/index?name=&locationType=&location=&type=&domain=&status=&startDate=&endDate=

As far as I can think, I see 2 ways to add this:

  • Implicitely, by declaring differently both behaviors:
#[LiveProp(writable: true, url: true)]
public string $search = ''; //Because still a string, param stays on URL

#[LiveProp(writable: true, url: true)]
public ?string $brand = null; //Because nullable, param disappears on URL if null
  • Explicitely, by adding an explicite "bool $hideEmptyParam" property to the Symfony\UX\LiveComponent\Metadata\UrlMapping class.

I feel like the first is more elegant but that's only brainstorming!

If I may help in any way,
Best regards,

@smnandre
Copy link
Member

Hello @Nayte91!

I'm not sure if this was 100% required or not (some comments in the code make me think it is... so poke @squrious ?)

If you feel to open some work on a PR, it would be a pleasure :)

(i would love if we could trim this query string 😅 )

@smnandre
Copy link
Member

Oh and i also think the first one makes the more sense

@squrious
Copy link
Contributor

Hi there! Yes, it's a design choice here, see this comment in the feature PR. I would also prefer to not show all unchanged parameters in the URL, this would be a great improvement.

But this is the tricky part: we would avoid props that didn't changed, not only those that are null, because a prop may be initialized with a default value on PHP side. I think the way Livewire handle this is pretty convenient: they use a keep option rather than hideEmpty, so by default all props that are in their initial state are not added to the URL, unless it's explicitly required in the prop definition.

@Nayte91 Nayte91 changed the title [LiveComponent] remove url query param when empty [LiveComponent] remove url query param when empty/default Sep 29, 2024
@Nayte91 Nayte91 changed the title [LiveComponent] remove url query param when empty/default [LiveComponent] Remove url query param when empty/default Sep 29, 2024
@Nayte91
Copy link
Author

Nayte91 commented Oct 2, 2024

But this is the tricky part: we would avoid props that didn't changed, not only those that are null, because a prop may be initialized with a default value on PHP side. I think the way Livewire handle this is pretty convenient: they use a keep option rather than hideEmpty, so by default all props that are in their initial state are not added to the URL, unless it's explicitly required in the prop definition.

I get it, it's all about what we write in the url if and only this is not the default param value. Not about the "empty string" case. LW solution seems OK. I'll try to make something around, or will ping here if it's too hard for me 🥷

1st though concern: having #[LiveProp(url:true, keep:true)] will begin to make a lot of parameter for such a common attribute. Should we us the regular controller's #[MapQueryString] to add this, or create a new #[LiveQueryString]one? Point is that if I'm right, we currently can't map query attributes into a DTO with LC, so I created an issue here. I feel like both, if functionally pertinent, will complexify too much the good old LiveProp, and therefore can need a new one (that will certainly extends #[LiveProp]).

@Nayte91 Nayte91 changed the title [LiveComponent] Remove url query param when empty/default [LiveComponent] Not write url query param when empty/default Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants