{"action":"created","issue":{"url":"https://api.github.com/repos/sonata-project/SonataAdminBundle/issues/3647","repository_url":"https://api.github.com/repos/sonata-project/SonataAdminBundle","labels_url":"https://api.github.com/repos/sonata-project/SonataAdminBundle/issues/3647/labels{/name}","comments_url":"https://api.github.com/repos/sonata-project/SonataAdminBundle/issues/3647/comments","events_url":"https://api.github.com/repos/sonata-project/SonataAdminBundle/issues/3647/events","html_url":"https://github.com/sonata-project/SonataAdminBundle/pull/3647","id":140429813,"number":3647,"title":"[wip] Refactors JS to remove inline scripts and centralize association handling.","user":{"login":"ju1ius","id":218404,"avatar_url":"https://avatars.githubusercontent.com/u/218404?v=3","gravatar_id":"","url":"https://api.github.com/users/ju1ius","html_url":"https://github.com/ju1ius","followers_url":"https://api.github.com/users/ju1ius/followers","following_url":"https://api.github.com/users/ju1ius/following{/other_user}","gists_url":"https://api.github.com/users/ju1ius/gists{/gist_id}","starred_url":"https://api.github.com/users/ju1ius/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ju1ius/subscriptions","organizations_url":"https://api.github.com/users/ju1ius/orgs","repos_url":"https://api.github.com/users/ju1ius/repos","events_url":"https://api.github.com/users/ju1ius/events{/privacy}","received_events_url":"https://api.github.com/users/ju1ius/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":13,"created_at":"2016-03-13T00:13:20Z","updated_at":"2016-03-13T16:37:54Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/sonata-project/SonataAdminBundle/pulls/3647","html_url":"https://github.com/sonata-project/SonataAdminBundle/pull/3647","diff_url":"https://github.com/sonata-project/SonataAdminBundle/pull/3647.diff","patch_url":"https://github.com/sonata-project/SonataAdminBundle/pull/3647.patch"},"body":"Hi ! :facepunch: \r\n\r\n| Q  |  A |\r\n------------ | -------------\r\n| Branch | master |\r\n| Bugfix? | yes |\r\n| New feature? | yes |\r\n| BC breaks | PHP: no, JS: yes, Twig: yes |\r\n| Tests pass? |\tPHP: yes, JS: well... |\r\n| Fixed tickets | #3646, #3173, #1076, [SonataDoctrineORMAdminBundle/#522](https://github.com/sonata-project/SonataDoctrineORMAdminBundle/issues/522), maybe others |\r\n\r\nThis PR, along with [it's companion for SonataDoctrineORMAdminBundle](https://github.com/sonata-project/SonataDoctrineORMAdminBundle/pull/526):\r\n\r\n* Adds the missing error handling and progress visual feedback to the AJAX requests (fixes [SonataDoctrineORMAdminBundle/#522](https://github.com/sonata-project/SonataDoctrineORMAdminBundle/issues/522))\r\n\r\n* Introduces [new, consistently named javascript events](https://github.com/sonata-project/SonataAdminBundle/pull/3647/files#diff-b295a634f9cd16cc294bf94526a42471R4) for custom code to hook into.\r\n\r\n* Introduces [public DOM helpers](https://github.com/sonata-project/SonataAdminBundle/pull/3647/files#diff-bf92b9c1651d01b45467998006f1f271R24) to interact with the sonata ui elements (still wip, we need to agree on a strong naming convention for css classes, see below).\r\n\r\n* Removes **all** inline, twig-entangled javascript from the templates in AdminBundle & SonataDoctrineORMAdminBundle. All data is now passed to javascript as JSON in data attributes, and  event delegation is used whenever possible.\r\n\r\n* Removes all the old javascript imperative Admin API (Admin.shared_setup, Admin.setup_my_plugin et al.). Instead, to initialize themselves, all plugins listen to the standard `DOMContentLoaded` event, and a custom `sonata:domready` jQuery event (triggered when a page is loaded via XMLHttpRequest).\r\n\r\n* Replaces the old \"show errors only in a popup\" form behavior.\r\nInstead, when errors are detected in a form: \r\n  * All tabs containing errors are clearly marked as such.\r\n  * The first tab containing an error is made active.\r\n  * The page is scrolled to the position of the first error.\r\n  * The offending field is focused.\r\n\r\nI didn't find the time to update the other doctrine admin bundles. However I'd like to have feedback on this.\r\nI've made a particular effort to document the code as much as possible, to make future public documentation work easier.\r\nThis lead me to point out a few inconsistencies in the current client-side/templating codebase.\r\n\r\n* CSS class naming is inconsitent. We should decide on a convention and stick to it. Currently some classes are prefixed with `.sonata-ba-*` (`.sonata-ba-action`, `.sonata-ba-form`), some are not (`.inner-field-short-description`...), and some parts use BEM (`.page-composer__page-preview__container`...). This makes javascript/css hacking in Sonata very fragile, and the codebase hard to understand. While I tried to address a few of these inconsistencies in this PR, there will be more work to do, across all bundles. (I propose we use BEM!)\r\n* A lot of twig templates are hard to read. I'm not a code-style freak, but some lines don't even fit on a 1920px wide monitor. Again this makes code hard to understand.\r\n* I question myself about why the various doctrine related bundles contain templates. Once all the JS logic is moved to the AdminBundle, which this PR does, could all the templates move here as well ?\r\n\r\nThanks for taking the time to review this."},"comment":{"url":"https://api.github.com/repos/sonata-project/SonataAdminBundle/issues/comments/195991499","html_url":"https://github.com/sonata-project/SonataAdminBundle/pull/3647#issuecomment-195991499","issue_url":"https://api.github.com/repos/sonata-project/SonataAdminBundle/issues/3647","id":195991499,"user":{"login":"ju1ius","id":218404,"avatar_url":"https://avatars.githubusercontent.com/u/218404?v=3","gravatar_id":"","url":"https://api.github.com/users/ju1ius","html_url":"https://github.com/ju1ius","followers_url":"https://api.github.com/users/ju1ius/followers","following_url":"https://api.github.com/users/ju1ius/following{/other_user}","gists_url":"https://api.github.com/users/ju1ius/gists{/gist_id}","starred_url":"https://api.github.com/users/ju1ius/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ju1ius/subscriptions","organizations_url":"https://api.github.com/users/ju1ius/orgs","repos_url":"https://api.github.com/users/ju1ius/repos","events_url":"https://api.github.com/users/ju1ius/events{/privacy}","received_events_url":"https://api.github.com/users/ju1ius/received_events","type":"User","site_admin":false},"created_at":"2016-03-13T16:37:54Z","updated_at":"2016-03-13T16:37:54Z","body":"Concerning BC:\r\nMany dynamic languages do not support the public/protected/private distinction.\r\nIn Python, the convention is that anything that is not _documented_ is not part of the public API. Things like prefixing variables and methods with an underscore are just considered as hints to the developer to use it at their own risk. So as the javascript API was not documented, one might say there's technically no BC break.... And since we're just initializing some plugins, [this might be enough](https://github.com/sonata-project/SonataAdminBundle/pull/3647/files#diff-bf92b9c1651d01b45467998006f1f271R193) for keeping BC.\r\n\r\nChanging the twig templates is more problematic though, since people could rely on the html structure for their custom code. (I do).\r\nAnd upgrading to select2 v4 will definitely break BC since there will be changes in the `ModelAutocompleteType` class."}}