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

Data attributes for layers #2827

Closed
wants to merge 1 commit into from
Closed

Conversation

DarKsandr
Copy link
Contributor

Done to assign a data controller for the layer. Subsequently use the Stimulus.

Example:

class PlatformScreen extends Screen
{
    ...
    public function layout(): iterable
    {
        return [
            Layout::rows([
                Input::make('name')->set('data-hello-target', 'name'),
            ])->setDataAttributes([
                'controller' => 'hello',
            ]),
        ];
    }
}

@DarKsandr
Copy link
Contributor Author

@tabuna

@tabuna
Copy link
Member

tabuna commented May 8, 2024

Hello, @DarKsandr! Thank you for the ping. Could you share some details about the task? I feel like this change might not significantly simplify the code for reuse. If we're talking about applying it only on one screen, perhaps we could achieve the same effect by adding a method to this screen's class:

public function frontEndController() : string {
   return 'hello';
}

This approach would allow us to apply the changes to the entire form or even page. What do you think about this idea?

@DarKsandr
Copy link
Contributor Author

@tabuna This is a good idea, but as an addition to mine. What if I want to assign different controllers to different layers? Let's say there is a filter and a table with data on the page: there will be one controller for the filter, and another for the table.

@tabuna
Copy link
Member

tabuna commented May 9, 2024

Usually, I create my own layers for display, which is not difficult at all. Also, we already have a wrapper that allows wrapping layers in our HTML elements. One of the universal options could be creating a Layout:stimulus layer that wraps content in a div with preliminary data. Since it's not enough to just declare the controller, it's also necessary to pass values ​​to it.

Layout::stimulus('hello', [SimpleRow::class], ['url' => 'keyForQuery', 'model' => 'keyForQuery'])

And the result:

<div data-controller="hello" 
    data-hello-url-value="http://example.com"
    data-hello-model-value='{json}'
>
   <!-- ... -->
</div>

We could also make the third argument optional, so that no values ​​are passed to the controller, and make the second argument act like a modal window, allowing either an array or a single value to be passed.

Layout::stimulus('hello', SimpleRow::class)

Furthermore, we could use the same names for both front-end and back-end values:

Layout::stimulus('hello', SimpleRow::class, ['url', 'model'])

@DarKsandr, sorry for the delay in responding, as I'm currently observing national holidays. What do you think about it?

@DarKsandr
Copy link
Contributor Author

@tabuna I assumed that the wrapper requires a blade template. I wanted to give attributes to layers, just as this has already been done for elements (set method). It’s also not very clear why there is an additional layer just for the data attribute? and yes, I also celebrate May 9th (С Днём Победы)

@DarKsandr
Copy link
Contributor Author

@tabuna Did you decide anything about the implementation?

@tabuna
Copy link
Member

tabuna commented May 22, 2024

Hi @DarKsandr ! If we start explicitly defining attributes for each layout, we may find it difficult to stop later on.

For example, currently, there's support for data attributes, and then we might add our own id or class. This could result in a mixture of JavaScript, CSS, and PHP in one file.

One of the main reasons is also that this approach does not promote code reuse. It's unlikely that you'll be able to reuse what's written this way due to the complexity of documentation and support.

Let me clarify this point. Currently, there aren't many extensions (Layout/Fields) in the community that we could use. If you create a JavaScript controller for fields (similar to UTM), it may be possible to not only reuse it, but also share it with others. However, if it's divided into more files, the likelihood of it being documented, supported, and looking good decreases (in my opinion).

I would prefer not to increase what's called the "reason for changes". I understand that you're referring to the experience with Field, but I see some issues with it that sooner or later will require refactoring.

If you're interested, I can provide a simple example (which doesn't affect PR and only leads to the fact that Field has problems):

<div class="">
    <input name="start">
    <input name="end">
</div>

Which of these three elements should data attributes be applied to? There's no good answer to this question, as we can add attributes to the div to control selection change, but then we lose the ability to set stimulus target. This example illustrates that I've made many mistakes, and I wouldn't want you to repeat them.

If you have a situation where you can't create a component and instead are trying to configure something, I'll try to help, and together we can find a good solution. If you're unable to disclose any information publicly, we can discuss it via private messages on Telegram.

@DarKsandr DarKsandr closed this by deleting the head repository May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants