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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vue3: Use slot value directly if it's a string in source decorator #23784

Merged
merged 7 commits into from Aug 23, 2023

Conversation

nasvillanueva
Copy link
Contributor

@nasvillanueva nasvillanueva commented Aug 10, 2023

What I did

Support plain text for "slot" controls.

Currently, only functions () => "Default Text Content", VNode h("p", "Default Text Content"), and stringified objects are supported.

and even if we do use those supported ways, the control table will display something like

// Function, renders as `Default Text Content`
default: {
}

// VNode, renders as `<p>Default Text Content</p>`
footer: {
  __v_isVNode: true,
  ...,
  type: "p",
  ...
  children: "Default Text Content",
  ...,
}

// Object, not even renders, but shows `{ a: 1, b: 2 }` in the "Show Code"
header: {
  a: 1,
  b: 2,
}

But I want users to be able to change the slot content, if it's only a label, so if the slot content is a string, we just render that instead.

ALSO, without this change, if you use a String slot content, on the "Show Code" block, you'll only see an empty template tag.

<template #default></template>

How to test

  1. Run a sandbox for template. I ran yarn task --task sandbox --start-from auto --template vue3-vite/default-ts
  2. Open Storybook in your browser
  3. Access Stories > renderers > vue3_vue3-vite-default-ts > Reactive Slots > Docs
  4. See aforementioned behavior on the slots controls.
  5. Edit any of the slots by clicking Raw button and Updating it with a double quoted string.
  6. Unfocus on the input
  7. The canvas should reflect the current string value, including the "Show Code" block.

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
    I don't know where to write tests, the slot function isn't tested anywhere.
  • Make sure to add/update documentation regarding your changes
    I don't think it is documented anywhere, but if I missed it, please let me know.
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "build", "documentation", "maintenance", "dependencies", "other"]

馃 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

@nasvillanueva
Copy link
Contributor Author

0ec0e0e is an opinionated commit. Happy to drop it if it's not relevant to the project.

@chakAs3
Copy link
Contributor

chakAs3 commented Aug 14, 2023

0ec0e0e is an opinionated commit. Happy to drop it if it's not relevant to the project.

It is relevant, however i guess we already have this let me checkout your PR and get back to you

@nasvillanueva
Copy link
Contributor Author

The CI seems to fail, please let me know if I need to do anything.

Not sure what to do with:

An error occurred while executing: touch yarn.lock,touch .yarnrc.yml,yarn set version berry,yarn config set enableGlobalCache true,yarn config set checksumBehavior ignore,yarn config set nodeLinker node-modules
馃毃 Installing Yarn 2 failed

@kasperpeulen kasperpeulen changed the title Use slot value directly if it's a string Vue3: Use slot value directly if it's a string in source decorator Aug 15, 2023
@kasperpeulen kasperpeulen self-assigned this Aug 15, 2023
Copy link
Contributor

@chakAs3 chakAs3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call. i did not handle this case previous cause we were supposed to pass a function only

@kasperpeulen
Copy link
Contributor

@yannbf Do you still want to look at how to test this PR?

@kasperpeulen kasperpeulen merged commit f8bcec8 into storybookjs:next Aug 23, 2023
45 checks passed
@nasvillanueva nasvillanueva deleted the text_slot branch August 23, 2023 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants