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

Introduce stimulus_controller to ease Stimulus Values API usage #109

Merged
merged 1 commit into from
Feb 10, 2021

Conversation

tgalopin
Copy link
Contributor

@tgalopin tgalopin commented Feb 9, 2021

This PR introduces a stimulus_controller helper to ease the usage of Stimulus Values API.

The Values API is very useful as it allows developers to store state on the DOM node and thus persist it, share it across controllers and build reusable controllers. Using the Values API as a way to configure a controller using data attributes is a very common pattern that we should encourage ("build primitive reusable and composable controllers instead of building a controller per page").

However it has a few drawbacks:

1/ It introduces a duplication of the scoped attributes:

<div data-controller="symfony--ux-chartjs-chart"
    data-symfony--ux-chartjs-chart-data-value="..."
    data-symfony--ux-chartjs-chart-options-value="..."
....>
</div>

This can quickly become cumbersome and difficult to track.

2/ It creates its own convention on naming, if you declare a value using camel case, it will need to be passed using kebab case in the HTML:

export default class extends Controller {
    static values = {
        myMessage: String,
    }

    connect() {
        this.element.textContent = this.myMessageValue;
    }
}
<div data-controller="hello" data-hello-my-message-value="Hello"></div>

3/ It requires to manually encode nested structure in JSON in order to pass them to the controller

This PR improves upon these drawbacks: it introduces a helper you can use in your DOM node to render the controller to use, with optional values for this controller. These values will be encoded properly to be read by Stimulus natively (https://stimulus.hotwire.dev/reference/values#properties-and-attributes).

With a single controller:

<div {{ stimulus_controller({ 'chart': { 'data': [1, 2, 3, 4] } }) }}>
    Hello
</div>

With multiple controllers:

<div {{ stimulus_controller({
    'chart': {
        'data': [1, 2, 3, 4],
        'labels': ['January', 'February', 'March', 'April'],
    },
    'another-controller': {}
}) }}>
    Hello
</div>

It introduces a "standard" way to transfer data to Stimulus controllers that map well with the Values API and thus work out of the box.

@tgalopin tgalopin force-pushed the stimulus-helper branch 5 times, most recently from 4b368e6 to 118a7aa Compare February 9, 2021 17:30
Copy link
Member

@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

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

I love this! Just minor comments/questions

src/Exception/MissingPackageException.php Outdated Show resolved Hide resolved
*
* @see https://stimulus.hotwire.dev/reference/controllers
*/
private function normalizeControllerName(string $str): string
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this belongs here. It seems to me that the UX packages should be responsible for calling this function with their already-normalized name. I'd almost rather throw an exception if something invalid is passed (but probably, just do nothing).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea was to allow users to pass the same value they have in their controller.json file: @symfony/ux-dropzone/dropzone, and this would be transformed to the proper HTML equivalent. But perhaps should we have an alternate way of doing this?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, that's a really good point. I'd like to keep it then for DX.

@weaverryan
Copy link
Member

This is awesome! Thank you Tito!

@weaverryan weaverryan merged commit ba6c670 into symfony:main Feb 10, 2021
@tgalopin tgalopin deleted the stimulus-helper branch November 30, 2021 14:42
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