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

feat: Allow localDev for direct CSS, JS assets mapping #33

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

theodorosploumis
Copy link

@theodorosploumis theodorosploumis commented Jun 6, 2023

Allows a CSS developer to use the local CSS/JS assets (filesystem) inside Storybook instead of the Drupal remote url paths. This allows local development for CSS/JS somehow. Notice that twig files still come from the (remote) Drupal url.

For example:

<link rel="stylesheet" media="all" href="https://my-ocal.ddev.site/themes/custom/my_theme/css/components/messages.css?rvtpha">

will become

<link rel="stylesheet" media="all" href="/themes/custom/my_theme/css/components/messages.css?rvtpha">

and will point to the local CSS assets from Drupal root.

  • A. Default remote Drupal url (example is using ddev)
    default_ddev

  • B. Local Drupal path from filesystem
    local_dev

@e0ipso
Copy link
Member

e0ipso commented Jun 6, 2023

Thanks for this @theodorosploumis! Could you explain the benefits of this PR?

I see you mention:

This allows local development for CSS/JS somehow

Can you elaborate a bit more on that?

@theodorosploumis
Copy link
Author

Well, in our case we need to allow CSS themers to work independendly from local Drupal installation. So we point Storybook server URL to a remote (online) domain but CSS Themers can still work locally and compile their local CSS/JS.

server: {
    url: "https://online-drupal-example.com/"
  },

I know that we could use another theming system like wingsuit or gesso but then we should loose all the magic from the new sdc module.

@e0ipso
Copy link
Member

e0ipso commented Jun 8, 2023

I understand better now.

Will this work if the server URL contains a subdirectory? I am thinking about something like:

  server: {
    url: "https://online-drupal-example.com/subdir/foo"
  },

In any case, I want to wait and see if others find this feature useful before committing to maintain it. I believe you can use it via the canary package.

@theodorosploumis
Copy link
Author

Right, in order to make this work I have to add the Drupal web root for the staticDirs.

So paths like http://localhost:6006/themes/.../.../my.css are valid.

// .storybook/main.js

module.exports = {
  stories: [
    ...
  ],
  staticDirs: ['../web'],
  addons: [
    ...
    "@lullabot/storybook-drupal-addon",
  ],
  framework: {
    name: "@storybook/server-webpack5",
    options: {}
  }
};

@theodorosploumis
Copy link
Author

@e0ipso A canary release would be great! Thanks.

@cosmicdreams
Copy link

cosmicdreams commented Jun 9, 2023

This KINDA looks like it would help me work around the CORS issue I have in pokemon_card for the pokemon.js it has

I'm eager to try this

@e0ipso
Copy link
Member

e0ipso commented Nov 24, 2023

I like the idea of this, but I think the implementation complexity to account for all cases is too high. This assumes a particular setup, and feature set, that if you step away from things stop working.

Things like where the Storybook instance is installed, weather the Drupal site is installed in a subdir, like https://www.example.org/subdir, multisite support (we are hardcoding /sites/default), support for images referenced from CSS, support for assets added conditionally via JS, etc.

I am inclined to leave this open, so other people can benefit from this work if they fit this use case. However, I am hesitant to merge it since it will likely end up being a source of feature requests, and support request.

My preferred course of action is that this is moved to a separate addon, so it can be independent of this. I would be open to emitting events so other addons can react to the HTML Drupal returns.

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