SonataAdminBundle icon indicating copy to clipboard operation
SonataAdminBundle copied to clipboard

Remove x-editable without any replacement

Open core23 opened this issue 2 years ago • 4 comments
trafficstars

Subject

I am targeting this branch, because feature removal is a breaking change.

Relates to https://github.com/sonata-project/SonataAdminBundle/issues/7156

Changelog

### Removed
- Remove `x-editable` without any replacement

The library is no longer maintained and this blocks the overall (frontend) upgrade process.

core23 avatar Sep 24 '23 15:09 core23

IMO we should make sure to deprecate this stuff on 4.x. (Also would be nice to do it before this PR, to avoid conflicts on upmerge)

jordisala1991 avatar Sep 26 '23 09:09 jordisala1991

I looked through the x-editable code and sonata-admin code. It is possible to replace x-editable by own js. We have something simmilar in ModelListType (with add button).

Also IMHO we should avoid using js in *.html.twig files and move current js to stimulus.

My solution

  • deprecate unnesessery js
  • add new stimulus controller equal to js from templates
  • add sonata_admin.options.js_type: inline(default)|stimulus(dedicated to 5.x)

Result

  • no inline JS in 5.x branch
  • JS will be independed with templates

BTW I can wrote stimulus to replace x-editable.

wbloszyk avatar Sep 26 '23 11:09 wbloszyk

I looked through the x-editable code and sonata-admin code. It is possible to replace x-editable by own js. We have something simmilar in ModelListType (with add button).

Also IMHO we should avoid using js in *.html.twig files and move current js to stimulus.

My solution

  • deprecate unnesessery js
  • add new stimulus controller equal to js from templates
  • add sonata_admin.options.js_type: inline(default)|stimulus(dedicated to 5.x)

Result

  • no inline JS in 5.x branch
  • JS will be independed with templates

Totally agree, we should use stimulus or web components where possible.

So should we deprecate all editable related components and add a new solution later on or should we directly replace it?

core23 avatar Sep 26 '23 15:09 core23

We talk about: ListMapper::add(string $name, ?string $type = null, array $fieldDescriptionOptions = []): static.
$fieldDescriptionOptions['editable'] are use in 4 places. (3 times in templates and 1 in SetObjectFieldAction.php).

SetObjectFieldAction allow for AJAX POST call only, so it ready backend endpoint for update field. x-editable is used for generate form (I dont see call for getting form in devTools) and send it to SetObjectFieldAdmin. So x-editable is only frontend thing.

IMO:

  • we should not remove x-editable.
  • We should deprecate whole template and replace it by other

I think about How to introduce new temple(s). The best solution for me will be improve current system. Pool for global templates registry and allow add own registry will be powerfull thing. With shwicher user could be able to change template in site. Downside of this solution will be overrided Admin templates, but it will be possible to fix/improve.

wbloszyk avatar Sep 28 '23 12:09 wbloszyk

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Mar 26 '24 13:03 github-actions[bot]