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

Allow for user defined query parameters #496

Closed
getinnocuous opened this issue Sep 18, 2023 · 12 comments
Closed

Allow for user defined query parameters #496

getinnocuous opened this issue Sep 18, 2023 · 12 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@getinnocuous
Copy link

Is your feature request related to a problem? Please describe.
Currently whenever you add any custom query parameters, they get removed by Ladle. For example, opening a modal via a query param of "?modalOpen=1"

Describe the solution you'd like
It would be great to allow custom query parameters per story that would be present on initial load of the story, as well as being able to update/add/remove parameters

Describe alternatives you've considered
I've tried adding the query parameters via a useEffect in the components.tsx like below but it doesn't work.

useEffect(() => {
  if (queryParams) {
    const urlParams = new URLSearchParams(window.location.search)
    Object.entries(queryParams).forEach((e) => {
      urlParams.set(e[0], e[1])
    })
  }
}, [queryParams])
@getinnocuous getinnocuous added the needs triage needs to be reviewed label Sep 18, 2023
@tajo tajo added enhancement New feature or request and removed needs triage needs to be reviewed labels Sep 18, 2023
@tajo
Copy link
Owner

tajo commented Sep 18, 2023

This could be pretty straightforward to implement - Ladle should preserve existing non-ladle params.

Relevant code: https://github.com/tajo/ladle/blob/main/packages/ladle/lib/app/src/history.ts#L25

@tajo tajo added the good first issue Good for newcomers label Sep 18, 2023
@cm-dyoshikawa
Copy link
Contributor

Could I work on it?

@tajo
Copy link
Owner

tajo commented Sep 19, 2023

Of course!

@cm-dyoshikawa
Copy link
Contributor

Thanks. I will research it.

@cm-dyoshikawa
Copy link
Contributor

cm-dyoshikawa commented Sep 21, 2023

@tajo

Probably what @getinnocuous wants is a mechanism that allows setting query parameters for each Story. If implemented, it could make the cataloging of components that depend on the value of the query parameter more effective.

There exists an Addon in Storybook that realizes it. So, how about adding an Addon that can set query parameters in Ladle as well?

It will provide the following API:

import { Story } from "@ladle/react";

export const Hello: Story = () => {
  const urlSearchParams = new URLSearchParams(document.location.search);
  return (
    <>
      <h1>Hello Query Parameters</h1>
      <p>{urlSearchParams.get("foo")}</p>
    </>
  );
};

Hello.meta = {
  // Set query params
  query: {
    foo: "bar",
  },
};

@tajo
Copy link
Owner

tajo commented Sep 21, 2023

I think @getinnocuous just wants to be able to programmatically set query params (aka their stories are reading/setting query params). Ladle now strips them away on any action - it should preserve them unless they collide with Ladle's params.

The add-on you describe is a different use-case. Not sure if its that useful since you can just use useEffect() to initialize query params.

@cm-dyoshikawa
Copy link
Contributor

OK. I considering.

@cm-dyoshikawa
Copy link
Contributor

Changing the code in the following way seems to make the combination of useEffect and history.pushState work

export const modifyParams = (globalState: GlobalState) => {
  if (!globalState.controlInitialized) return;
  const params = {
+   ...queryString.parse(location.search),
    mode: globalState.mode,
    rtl: globalState.rtl,
    source: globalState.source,
    story: globalState.story,
    theme: globalState.theme,
    width: globalState.width,
    control: globalState.control,
  };

Sample code for using the library:

import type { Story } from "@ladle/react";
import { useEffect, useState } from "react";

export const CardTest: Story = () => {
  const [queryParams, setQueryParams] = useState<string>("");

  useEffect(() => {
    setQueryParams(location.search);
  }, []);

  return <p>Params: {queryParams}</p>;
  // Expected => "Params: ?story=query-parameters--card-test&foo=bar"
  // Actual => "Params: ?story=query-parameters--card-test"
};

CardTest.decorators = [
  (Component) => {
    useEffect(() => {
      const url = new URL(window.location.href);
      url.searchParams.set("foo", "bar");
      history.pushState({}, "", url);
    }, []);

    return (
      <>
        <Component />
      </>
    );
  },
];

However, in this case as well, it seems challenging to capture the URL query parameters after the component has changed. Most React components that use query parameters are likely expected to obtain their value at the initial rendering with useEffect(() => {}, []). But, there are cases where these changes may not be ready at the time of this acquisition.

I want your opinion.

@tajo
Copy link
Owner

tajo commented Sep 23, 2023

useEffect(() => {
  setQueryParams(location.search);
}, [location.search]);

would fix that?

@cm-dyoshikawa
Copy link
Contributor

Good point, thank you. I'll create the PR.

@tajo
Copy link
Owner

tajo commented Oct 3, 2023

landed as part of v3.2, thanks to @cm-dyoshikawa

@tajo tajo closed this as completed Oct 3, 2023
@getinnocuous
Copy link
Author

legends! great work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants