{"action":"created","comment":{"url":"https://api.github.com/repos/atk4/ui/pulls/comments/126487325","pull_request_review_id":48976254,"id":126487325,"diff_hunk":"@@ -0,0 +1,30 @@\n+<?php\n+\n+namespace atk4\\ui;\n+\n+class Modal extends View","path":"src/Modal.php","position":5,"original_position":5,"commit_id":"e53941f70ee3db9eda0352a851b2855ecf53a433","original_commit_id":"22f4e060168176ac4fc5f31fe403f4353de8aaab","user":{"login":"ibelar","id":2204478,"avatar_url":"https://avatars0.githubusercontent.com/u/2204478?v=3","gravatar_id":"","url":"https://api.github.com/users/ibelar","html_url":"https://github.com/ibelar","followers_url":"https://api.github.com/users/ibelar/followers","following_url":"https://api.github.com/users/ibelar/following{/other_user}","gists_url":"https://api.github.com/users/ibelar/gists{/gist_id}","starred_url":"https://api.github.com/users/ibelar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ibelar/subscriptions","organizations_url":"https://api.github.com/users/ibelar/orgs","repos_url":"https://api.github.com/users/ibelar/repos","events_url":"https://api.github.com/users/ibelar/events{/privacy}","received_events_url":"https://api.github.com/users/ibelar/received_events","type":"User","site_admin":false},"body":"Here the things with Modal as a VirtualPage. Correct me if I am wrong but VP does not output any content to DOM until they are called and $modal->js()->modal('show') need to have the $modal element in place in DOM for it to work properly. \r\nWhich mean that if we put Modal as a derived class of VirutalPage, we will need to add the modal html element to the DOM prior to call modal('show'), which mean using createModal.js plugin again and this is already use by jsModal.php. \r\n\r\nWhat if we change the method name to \r\n```\r\n$modal->addVirtualPage($vp) \r\n```\r\ninstead of\r\n\r\n```\r\n$modal->setUri($vp->getUrl('cut'))\r\n```\r\nWe could do the proper wiring of setUrl inside AddVirtualPage method and it would probably make more sense to developers using it. \r\n\r\nLet me know your thoughts","created_at":"2017-07-10T17:34:05Z","updated_at":"2017-07-10T17:34:05Z","html_url":"https://github.com/atk4/ui/pull/194#discussion_r126487325","pull_request_url":"https://api.github.com/repos/atk4/ui/pulls/194","_links":{"self":{"href":"https://api.github.com/repos/atk4/ui/pulls/comments/126487325"},"html":{"href":"https://github.com/atk4/ui/pull/194#discussion_r126487325"},"pull_request":{"href":"https://api.github.com/repos/atk4/ui/pulls/194"}}},"pull_request":{"url":"https://api.github.com/repos/atk4/ui/pulls/194","id":129453958,"html_url":"https://github.com/atk4/ui/pull/194","diff_url":"https://github.com/atk4/ui/pull/194.diff","patch_url":"https://github.com/atk4/ui/pull/194.patch","issue_url":"https://api.github.com/repos/atk4/ui/issues/194","number":194,"state":"open","locked":false,"title":"Feature/js modal rev","user":{"login":"ibelar","id":2204478,"avatar_url":"https://avatars0.githubusercontent.com/u/2204478?v=3","gravatar_id":"","url":"https://api.github.com/users/ibelar","html_url":"https://github.com/ibelar","followers_url":"https://api.github.com/users/ibelar/followers","following_url":"https://api.github.com/users/ibelar/following{/other_user}","gists_url":"https://api.github.com/users/ibelar/gists{/gist_id}","starred_url":"https://api.github.com/users/ibelar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ibelar/subscriptions","organizations_url":"https://api.github.com/users/ibelar/orgs","repos_url":"https://api.github.com/users/ibelar/repos","events_url":"https://api.github.com/users/ibelar/events{/privacy}","received_events_url":"https://api.github.com/users/ibelar/received_events","type":"User","site_admin":false},"body":"# Javascript and Modal Revision\r\n\r\n## Change reason\r\nLearning semantic-ui furthermore allow me to better understand the semantic module settings and behaviours. These changes will take more advantage of semantic-ui functionality now and in future. For example, modal are now using semantic api call in order to load their dynamic content. By using a service to control them, we can also implement our own behaviour to them. Loading content to api module also allow for better error control. Finally, modal can now be add to DOM directly using the new Modal.php class.\r\n## Javascript\r\n### Folder structure\r\nBetter structure the javascript file by making them more modular. The folder structure is as follow\r\n- plugins: contains the jquery plugin file\r\n- helpers: contains extended jquery function, like $.addParams, perhaps eventually univ()...\r\n- service: contains mainly singleton class responsible to manage semantic-ui module, like the api or modal module throughout the entire app.\r\n\r\n### jQuery plugin enhancement\r\nBetter handling of jQuery plugin creation via plugin.js. Also now handle direct plugin function call using $('element').plugin('functionToCall', [param]) type syntax.\r\n\r\n### Service class\r\nTwo new service classes, ApiService and ModalService, for managing api or modal throughout the app.\r\n- ApiService: better handling of error message and api call done via other service, like modal;\r\n- ModalService: better handling of modal\r\n     - allow support for multiple modal, properly handling close trigger event.\r\n     - allow dynamic content via createModal or Modal.php.\r\n     - allow for regular content via Modal.php\r\n\r\nService can be add for other semantic-ui module like form, tab etc, in future.\r\n\r\n## PHP\r\n### Modal.php\r\nAdd Modal class in order to allow modal use within a page. \r\n\r\nThis is an example where clicking a button will add a modal that contains another button that will load a CRUD with full editing capability in modal as well. \r\n```\r\n$modal1 = $layout->add('Modal');\r\n$modal2 = $layout->add('Modal');\r\n\r\n$vp = $layout->add('VirtualPage'); // this page will not be visible unless you trigger it specifically\r\n$vp->add(['Header', 'Contens of your pop-up here']);\r\n$vp->add(['LoremIpsum', 'size'=>2]);\r\n$vp->add('Button')->set('Proceed')->on('click', $modal2->js()->modal('show'));\r\n\r\n$vp1 = $layout->add('VirtualPage'); // this page will not be visible unless you trigger it specifically\r\n$c = $vp1->add('CRUD');\r\n$c->setModel($m)\r\n\r\n$modal1->setUri($vp->getURL('cut'));\r\n$modal2->setUri($vp1->getURL('cut'));\r\n\r\n$bar = $layout->add(['View', 'ui'=>'buttons']);\r\n$b = $bar->add('Button')->set('Load Modal');\r\n\r\n$b->on('click', $modal1->js()->modal('show'));\r\n```\r\n\r\n    \r\n","created_at":"2017-07-07T17:44:13Z","updated_at":"2017-07-10T17:34:05Z","closed_at":null,"merged_at":null,"merge_commit_sha":"326039b3cae4e53252ae1a92e754aca9e9213a34","assignee":null,"assignees":[],"requested_reviewers":[],"milestone":null,"commits_url":"https://api.github.com/repos/atk4/ui/pulls/194/commits","review_comments_url":"https://api.github.com/repos/atk4/ui/pulls/194/comments","review_comment_url":"https://api.github.com/repos/atk4/ui/pulls/comments{/number}","comments_url":"https://api.github.com/repos/atk4/ui/issues/194/comments","statuses_url":"https://api.github.com/repos/atk4/ui/statuses/e53941f70ee3db9eda0352a851b2855ecf53a433","head":{"label":"ibelar:feature/js-modal-rev","ref":"feature/js-modal-rev","sha":"e53941f70ee3db9eda0352a851b2855ecf53a433","user":{"login":"ibelar","id":2204478,"avatar_url":"https://avatars0.githubusercontent.com/u/2204478?v=3","gravatar_id":"","url":"https://api.github.com/users/ibelar","html_url":"https://github.com/ibelar","followers_url":"https://api.github.com/users/ibelar/followers","following_url":"https://api.github.com/users/ibelar/following{/other_user}","gists_url":"https://api.github.com/users/ibelar/gists{/gist_id}","starred_url":"https://api.github.com/users/ibelar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ibelar/subscriptions","organizations_url":"https://api.github.com/users/ibelar/orgs","repos_url":"https://api.github.com/users/ibelar/repos","events_url":"https://api.github.com/users/ibelar/events{/privacy}","received_events_url":"https://api.github.com/users/ibelar/received_events","type":"User","site_admin":false},"repo":{"id":94804266,"name":"ui","full_name":"ibelar/ui","owner":{"login":"ibelar","id":2204478,"avatar_url":"https://avatars0.githubusercontent.com/u/2204478?v=3","gravatar_id":"","url":"https://api.github.com/users/ibelar","html_url":"https://github.com/ibelar","followers_url":"https://api.github.com/users/ibelar/followers","following_url":"https://api.github.com/users/ibelar/following{/other_user}","gists_url":"https://api.github.com/users/ibelar/gists{/gist_id}","starred_url":"https://api.github.com/users/ibelar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ibelar/subscriptions","organizations_url":"https://api.github.com/users/ibelar/orgs","repos_url":"https://api.github.com/users/ibelar/repos","events_url":"https://api.github.com/users/ibelar/events{/privacy}","received_events_url":"https://api.github.com/users/ibelar/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/ibelar/ui","description":"Web Component Framework written in PHP","fork":true,"url":"https://api.github.com/repos/ibelar/ui","forks_url":"https://api.github.com/repos/ibelar/ui/forks","keys_url":"https://api.github.com/repos/ibelar/ui/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ibelar/ui/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ibelar/ui/teams","hooks_url":"https://api.github.com/repos/ibelar/ui/hooks","issue_events_url":"https://api.github.com/repos/ibelar/ui/issues/events{/number}","events_url":"https://api.github.com/repos/ibelar/ui/events","assignees_url":"https://api.github.com/repos/ibelar/ui/assignees{/user}","branches_url":"https://api.github.com/repos/ibelar/ui/branches{/branch}","tags_url":"https://api.github.com/repos/ibelar/ui/tags","blobs_url":"https://api.github.com/repos/ibelar/ui/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ibelar/ui/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ibelar/ui/git/refs{/sha}","trees_url":"https://api.github.com/repos/ibelar/ui/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ibelar/ui/statuses/{sha}","languages_url":"https://api.github.com/repos/ibelar/ui/languages","stargazers_url":"https://api.github.com/repos/ibelar/ui/stargazers","contributors_url":"https://api.github.com/repos/ibelar/ui/contributors","subscribers_url":"https://api.github.com/repos/ibelar/ui/subscribers","subscription_url":"https://api.github.com/repos/ibelar/ui/subscription","commits_url":"https://api.github.com/repos/ibelar/ui/commits{/sha}","git_commits_url":"https://api.github.com/repos/ibelar/ui/git/commits{/sha}","comments_url":"https://api.github.com/repos/ibelar/ui/comments{/number}","issue_comment_url":"https://api.github.com/repos/ibelar/ui/issues/comments{/number}","contents_url":"https://api.github.com/repos/ibelar/ui/contents/{+path}","compare_url":"https://api.github.com/repos/ibelar/ui/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ibelar/ui/merges","archive_url":"https://api.github.com/repos/ibelar/ui/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ibelar/ui/downloads","issues_url":"https://api.github.com/repos/ibelar/ui/issues{/number}","pulls_url":"https://api.github.com/repos/ibelar/ui/pulls{/number}","milestones_url":"https://api.github.com/repos/ibelar/ui/milestones{/number}","notifications_url":"https://api.github.com/repos/ibelar/ui/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ibelar/ui/labels{/name}","releases_url":"https://api.github.com/repos/ibelar/ui/releases{/id}","deployments_url":"https://api.github.com/repos/ibelar/ui/deployments","created_at":"2017-06-19T17:42:42Z","updated_at":"2017-06-19T17:42:44Z","pushed_at":"2017-07-10T12:27:19Z","git_url":"git://github.com/ibelar/ui.git","ssh_url":"git@github.com:ibelar/ui.git","clone_url":"https://github.com/ibelar/ui.git","svn_url":"https://github.com/ibelar/ui","homepage":"","size":1807,"stargazers_count":0,"watchers_count":0,"language":"PHP","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"develop"}},"base":{"label":"atk4:develop","ref":"develop","sha":"957efbcc454093e9aa7d10229238241c90656147","user":{"login":"atk4","id":454075,"avatar_url":"https://avatars3.githubusercontent.com/u/454075?v=3","gravatar_id":"","url":"https://api.github.com/users/atk4","html_url":"https://github.com/atk4","followers_url":"https://api.github.com/users/atk4/followers","following_url":"https://api.github.com/users/atk4/following{/other_user}","gists_url":"https://api.github.com/users/atk4/gists{/gist_id}","starred_url":"https://api.github.com/users/atk4/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/atk4/subscriptions","organizations_url":"https://api.github.com/users/atk4/orgs","repos_url":"https://api.github.com/users/atk4/repos","events_url":"https://api.github.com/users/atk4/events{/privacy}","received_events_url":"https://api.github.com/users/atk4/received_events","type":"Organization","site_admin":false},"repo":{"id":68417565,"name":"ui","full_name":"atk4/ui","owner":{"login":"atk4","id":454075,"avatar_url":"https://avatars3.githubusercontent.com/u/454075?v=3","gravatar_id":"","url":"https://api.github.com/users/atk4","html_url":"https://github.com/atk4","followers_url":"https://api.github.com/users/atk4/followers","following_url":"https://api.github.com/users/atk4/following{/other_user}","gists_url":"https://api.github.com/users/atk4/gists{/gist_id}","starred_url":"https://api.github.com/users/atk4/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/atk4/subscriptions","organizations_url":"https://api.github.com/users/atk4/orgs","repos_url":"https://api.github.com/users/atk4/repos","events_url":"https://api.github.com/users/atk4/events{/privacy}","received_events_url":"https://api.github.com/users/atk4/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/atk4/ui","description":"Web Component Framework written in PHP","fork":false,"url":"https://api.github.com/repos/atk4/ui","forks_url":"https://api.github.com/repos/atk4/ui/forks","keys_url":"https://api.github.com/repos/atk4/ui/keys{/key_id}","collaborators_url":"https://api.github.com/repos/atk4/ui/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/atk4/ui/teams","hooks_url":"https://api.github.com/repos/atk4/ui/hooks","issue_events_url":"https://api.github.com/repos/atk4/ui/issues/events{/number}","events_url":"https://api.github.com/repos/atk4/ui/events","assignees_url":"https://api.github.com/repos/atk4/ui/assignees{/user}","branches_url":"https://api.github.com/repos/atk4/ui/branches{/branch}","tags_url":"https://api.github.com/repos/atk4/ui/tags","blobs_url":"https://api.github.com/repos/atk4/ui/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/atk4/ui/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/atk4/ui/git/refs{/sha}","trees_url":"https://api.github.com/repos/atk4/ui/git/trees{/sha}","statuses_url":"https://api.github.com/repos/atk4/ui/statuses/{sha}","languages_url":"https://api.github.com/repos/atk4/ui/languages","stargazers_url":"https://api.github.com/repos/atk4/ui/stargazers","contributors_url":"https://api.github.com/repos/atk4/ui/contributors","subscribers_url":"https://api.github.com/repos/atk4/ui/subscribers","subscription_url":"https://api.github.com/repos/atk4/ui/subscription","commits_url":"https://api.github.com/repos/atk4/ui/commits{/sha}","git_commits_url":"https://api.github.com/repos/atk4/ui/git/commits{/sha}","comments_url":"https://api.github.com/repos/atk4/ui/comments{/number}","issue_comment_url":"https://api.github.com/repos/atk4/ui/issues/comments{/number}","contents_url":"https://api.github.com/repos/atk4/ui/contents/{+path}","compare_url":"https://api.github.com/repos/atk4/ui/compare/{base}...{head}","merges_url":"https://api.github.com/repos/atk4/ui/merges","archive_url":"https://api.github.com/repos/atk4/ui/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/atk4/ui/downloads","issues_url":"https://api.github.com/repos/atk4/ui/issues{/number}","pulls_url":"https://api.github.com/repos/atk4/ui/pulls{/number}","milestones_url":"https://api.github.com/repos/atk4/ui/milestones{/number}","notifications_url":"https://api.github.com/repos/atk4/ui/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/atk4/ui/labels{/name}","releases_url":"https://api.github.com/repos/atk4/ui/releases{/id}","deployments_url":"https://api.github.com/repos/atk4/ui/deployments","created_at":"2016-09-16T22:06:53Z","updated_at":"2017-07-09T16:45:28Z","pushed_at":"2017-07-10T15:08:46Z","git_url":"git://github.com/atk4/ui.git","ssh_url":"git@github.com:atk4/ui.git","clone_url":"https://github.com/atk4/ui.git","svn_url":"https://github.com/atk4/ui","homepage":"","size":2193,"stargazers_count":25,"watchers_count":25,"language":"PHP","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":8,"mirror_url":null,"open_issues_count":56,"forks":8,"open_issues":56,"watchers":25,"default_branch":"develop"}},"_links":{"self":{"href":"https://api.github.com/repos/atk4/ui/pulls/194"},"html":{"href":"https://github.com/atk4/ui/pull/194"},"issue":{"href":"https://api.github.com/repos/atk4/ui/issues/194"},"comments":{"href":"https://api.github.com/repos/atk4/ui/issues/194/comments"},"review_comments":{"href":"https://api.github.com/repos/atk4/ui/pulls/194/comments"},"review_comment":{"href":"https://api.github.com/repos/atk4/ui/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/atk4/ui/pulls/194/commits"},"statuses":{"href":"https://api.github.com/repos/atk4/ui/statuses/e53941f70ee3db9eda0352a851b2855ecf53a433"}}}}
{"actor":{"display_login":"ibelar"}}