Skip to content

[StimulusBundle] Docs, replace chart examples by hello to avoid confusion with the ChartJS component #2707

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

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from

Conversation

welcoMattic
Copy link
Member

Q A
Bug fix? no
New feature? no
Docs? yes
Issues Fix #...
License MIT

As ChartJS component exists, the examples using chart as controller identifier are confusing. ChartJS component does not expect name and data as values, but view.

Examples has been updated with a dead simple hello world style, which can not be confusing.

@Kocal Kocal added the docs Improvements or additions to documentation label May 2, 2025
Copy link
Member

@Kocal Kocal left a comment

Choose a reason for hiding this comment

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

That's totally makes sense, thanks

@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels May 2, 2025
@smnandre
Copy link
Member

smnandre commented May 3, 2025

Thanks for pointing that! I 100% agree for the confusion, but... we try to make examples that do have some sense and avoid the foo bars when possible.

Maybe one of these:

  • a cart controller (classic example) with add action and productId ?
  • an alert with close ?
  • a notification with link and action ?

WDYT ?

@welcoMattic
Copy link
Member Author

@smnandre I would agree for advanced or "demo" examples (ones on ux.symfony.com). But on pure text documentation, I personally prefer to have easy to understand and dead simple examples like "Hello World". It's totally personal, if you prefer more advanced and real life example, I'm ok with that.

@smnandre
Copy link
Member

smnandre commented May 4, 2025

In fact, with its unusual syntax for some, Stimulus isn’t always easy to grasp.

The first examples here should be self-explanatory for most people, and I do believe we need to clearly emphasize what action, target, and controller mean.

If I read this, with the value name, I’m pretty sure it could lead to confusion:

<div {{ stimulus_controller('hello', { 'name': 'World', 'data': [1, 2, 3, 4] }) }}>

That said, your original point is totally valid. I’ll send you another suggestion later today.

Copy link
Member

@smnandre smnandre left a comment

Choose a reason for hiding this comment

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

Well there is a lot to change on this page...

A few very minor details on your PR and LGTM!

Thx @welcoMattic

@welcoMattic welcoMattic force-pushed the welcoMattic-patch-1 branch from 5b8eb5d to 77ee62e Compare May 5, 2025 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to documentation Status: Reviewed Has been reviewed by a maintainer StimulusBundle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants