{"action":"created","issue":{"url":"https://api.github.com/repos/vaadin/framework/issues/98","repository_url":"https://api.github.com/repos/vaadin/framework","labels_url":"https://api.github.com/repos/vaadin/framework/issues/98/labels{/name}","comments_url":"https://api.github.com/repos/vaadin/framework/issues/98/comments","events_url":"https://api.github.com/repos/vaadin/framework/issues/98/events","html_url":"https://github.com/vaadin/framework/pull/98","id":194020279,"number":98,"title":"Introduce dedicated shared states for every core vaadin component/ext.","user":{"login":"denis-anisimov","id":11425472,"avatar_url":"https://avatars.githubusercontent.com/u/11425472?v=3","gravatar_id":"","url":"https://api.github.com/users/denis-anisimov","html_url":"https://github.com/denis-anisimov","followers_url":"https://api.github.com/users/denis-anisimov/followers","following_url":"https://api.github.com/users/denis-anisimov/following{/other_user}","gists_url":"https://api.github.com/users/denis-anisimov/gists{/gist_id}","starred_url":"https://api.github.com/users/denis-anisimov/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/denis-anisimov/subscriptions","organizations_url":"https://api.github.com/users/denis-anisimov/orgs","repos_url":"https://api.github.com/users/denis-anisimov/repos","events_url":"https://api.github.com/users/denis-anisimov/events{/privacy}","received_events_url":"https://api.github.com/users/denis-anisimov/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"hesara","id":1335918,"avatar_url":"https://avatars.githubusercontent.com/u/1335918?v=3","gravatar_id":"","url":"https://api.github.com/users/hesara","html_url":"https://github.com/hesara","followers_url":"https://api.github.com/users/hesara/followers","following_url":"https://api.github.com/users/hesara/following{/other_user}","gists_url":"https://api.github.com/users/hesara/gists{/gist_id}","starred_url":"https://api.github.com/users/hesara/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hesara/subscriptions","organizations_url":"https://api.github.com/users/hesara/orgs","repos_url":"https://api.github.com/users/hesara/repos","events_url":"https://api.github.com/users/hesara/events{/privacy}","received_events_url":"https://api.github.com/users/hesara/received_events","type":"User","site_admin":false},"assignees":[{"login":"hesara","id":1335918,"avatar_url":"https://avatars.githubusercontent.com/u/1335918?v=3","gravatar_id":"","url":"https://api.github.com/users/hesara","html_url":"https://github.com/hesara","followers_url":"https://api.github.com/users/hesara/followers","following_url":"https://api.github.com/users/hesara/following{/other_user}","gists_url":"https://api.github.com/users/hesara/gists{/gist_id}","starred_url":"https://api.github.com/users/hesara/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hesara/subscriptions","organizations_url":"https://api.github.com/users/hesara/orgs","repos_url":"https://api.github.com/users/hesara/repos","events_url":"https://api.github.com/users/hesara/events{/privacy}","received_events_url":"https://api.github.com/users/hesara/received_events","type":"User","site_admin":false},{"login":"pleku","id":1478381,"avatar_url":"https://avatars.githubusercontent.com/u/1478381?v=3","gravatar_id":"","url":"https://api.github.com/users/pleku","html_url":"https://github.com/pleku","followers_url":"https://api.github.com/users/pleku/followers","following_url":"https://api.github.com/users/pleku/following{/other_user}","gists_url":"https://api.github.com/users/pleku/gists{/gist_id}","starred_url":"https://api.github.com/users/pleku/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pleku/subscriptions","organizations_url":"https://api.github.com/users/pleku/orgs","repos_url":"https://api.github.com/users/pleku/repos","events_url":"https://api.github.com/users/pleku/events{/privacy}","received_events_url":"https://api.github.com/users/pleku/received_events","type":"User","site_admin":false}],"milestone":null,"comments":3,"created_at":"2016-12-07T11:05:31Z","updated_at":"2016-12-07T11:56:13Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/vaadin/framework/pulls/98","html_url":"https://github.com/vaadin/framework/pull/98","diff_url":"https://github.com/vaadin/framework/pull/98.diff","patch_url":"https://github.com/vaadin/framework/pull/98.patch"},"body":"Updated components are:\r\n- PasswordField\r\n- CustomField\r\n- AbstractListing\r\n- AbstractMultiSelect\r\n- ColorPicker\r\n- ColorPickerArea\r\n- AbstractExtension\r\n- DataCommunicator\r\n- Responsive\r\n- AbstractListingExtension\r\n- DetailsManager\r\n- AbstractRenderer\r\n- ClickableRenderer\r\n- ButtonRenderer\r\n- CheckBoxRenderer\r\n- DateRenderer\r\n- HtmlRenderer\r\n- ImageRenderer\r\n- NumberRenderer\r\n- ProgressBarRenderer\r\n- TextRenderer\r\n\r\nFixes vaadin/framework8-issues#480.\n\n<!-- Reviewable:start -->\n---\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/vaadin/framework/98)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/vaadin/framework/issues/comments/265429100","html_url":"https://github.com/vaadin/framework/pull/98#issuecomment-265429100","issue_url":"https://api.github.com/repos/vaadin/framework/issues/98","id":265429100,"user":{"login":"pleku","id":1478381,"avatar_url":"https://avatars.githubusercontent.com/u/1478381?v=3","gravatar_id":"","url":"https://api.github.com/users/pleku","html_url":"https://github.com/pleku","followers_url":"https://api.github.com/users/pleku/followers","following_url":"https://api.github.com/users/pleku/following{/other_user}","gists_url":"https://api.github.com/users/pleku/gists{/gist_id}","starred_url":"https://api.github.com/users/pleku/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pleku/subscriptions","organizations_url":"https://api.github.com/users/pleku/orgs","repos_url":"https://api.github.com/users/pleku/repos","events_url":"https://api.github.com/users/pleku/events{/privacy}","received_events_url":"https://api.github.com/users/pleku/received_events","type":"User","site_admin":false},"created_at":"2016-12-07T11:56:13Z","updated_at":"2016-12-07T11:56:13Z","body":"\n\n\n\nReviewed 86 of 88 files at r1.\nReview status: 86 of 88 files reviewed at latest revision, 7 unresolved discussions, some commit checks failed.\n\n---\n\n*[a discussion](https://reviewable.io:443/reviews/vaadin/framework/98#-KYO2Z2B5100kZ1vwyIw:-KYO6Zq75es8BjppOJpO:b5a7nzg) (no related file):*\n<details><summary><i>Previously, denis-anisimov (Denis) wrote…</i></summary><blockquote>\n\nI can't do this because of the same reasons as with `FieldEvents` in a different patch.\n\nTwo compatibility states are used in the Grid extensions. Both extensions extend `AbstractExtension` which has now its own state. It means that extension have to have state which extends `AbstractExtensionState`.\nSo either that or let's remove `AbstractExtensionState`.\n\nIt's not that easy unfortunately.\n</blockquote></details>\n\nSo two options:\n1. remove `AbstractExtensionState` for now, and hope we'll never have something we would want to add to all `AbstractExtensionState`s. We can of course do that for 8.1 or so\n2. duplicate the necessary states for compatibility packages.\n\nI think we should do 2) for all the cases since effected we don't want to break the compatibility components when we add the shared states to V8 components.\n\n---\n\n*[server/src/main/java/com/vaadin/ui/AbstractListing.java, line 52 at r1](https://reviewable.io:443/reviews/vaadin/framework/98#-KYO4ca5AZeuqc90hX__:-KYO4ca59P9eNOC5wjVj:bxfp204) ([raw file](https://github.com/vaadin/framework/blob/5b0919ce9e3f8899fb5b0c5331afd9ffa2a56c0c/server/src/main/java/com/vaadin/ui/AbstractListing.java#L52)):*\n> ```Java\n>  * @see Listing\n>  */\n> public abstract class AbstractListing<T> extends AbstractComponent {\n> ```\n\nAbstractListing doesn't extend AbstractField so how can AbstractListingState extend AbstractFieldState ?\n\n---\n\n*[shared/src/main/java/com/vaadin/shared/JavaScriptExtensionState.java, line 26 at r1](https://reviewable.io:443/reviews/vaadin/framework/98#-KYO3Mv99XbWJQh1gZGB:-KYO3Mv92yW_o2T21liM:bkr4z93) ([raw file](https://github.com/vaadin/framework/blob/138d7cac6a10f32f61a4fcee27d7b2eca02f7494/shared/src/main/java/com/vaadin/shared/JavaScriptExtensionState.java#L26)):*\n> ```Java\n> import com.vaadin.shared.ui.grid.renderers.AbstractRendererState;\n> \n> public class JavaScriptExtensionState extends AbstractRendererState\n> ```\n\nThis should be renamed to AbstractJavaScriptExtensionState, but only if it is not used by any V7 component.\nOr maybe just keep it as is since, while it is wrong currently, it might break things.\n\n---\n\n*[shared/src/main/java/com/vaadin/shared/extension/abstractlisting/AbstractListingExtensionState.java, line 21 at r1](https://reviewable.io:443/reviews/vaadin/framework/98#-KYO3qUI0gGofsS_FlFc:-KYO3qUI38WkOU63OXtB:bg3roar) ([raw file](https://github.com/vaadin/framework/blob/138d7cac6a10f32f61a4fcee27d7b2eca02f7494/shared/src/main/java/com/vaadin/shared/extension/abstractlisting/AbstractListingExtensionState.java#L21)):*\n> ```Java\n> \n> /**\n>  * Default shared state implementation for AbstractListingExtension.\n> ```\n\nDon't think it is required to say \"default shared state for X\" instead of just \"shared state for X\", which applies to all the shared states. This is not blocking any way\n\n---\n\n*[shared/src/main/java/com/vaadin/shared/ui/AbstractSingleSelectState.java, line 27 at r1](https://reviewable.io:443/reviews/vaadin/framework/98#-KYO4E9r7Tx-yRck4x2C:-KYO4E9r9UsRd9wq4bJf:biknz42) ([raw file](https://github.com/vaadin/framework/blob/5b0919ce9e3f8899fb5b0c5331afd9ffa2a56c0c/shared/src/main/java/com/vaadin/shared/ui/AbstractSingleSelectState.java#L27)):*\n> ```Java\n>  * @since 8.0\n>  */\n> public class AbstractSingleSelectState extends AbstractFieldState {\n> ```\n\nDoes this break things ? AbstractFieldState has required and read only \n\n---\n\n*[shared/src/main/java/com/vaadin/shared/ui/abstractlisting/AbstractListingState.java, line 27 at r1](https://reviewable.io:443/reviews/vaadin/framework/98#-KYO4qHcCY0z0-wa08A_:-KYO4qHcD56mhpccT9ns:b-7qanqw) ([raw file](https://github.com/vaadin/framework/blob/138d7cac6a10f32f61a4fcee27d7b2eca02f7494/shared/src/main/java/com/vaadin/shared/ui/abstractlisting/AbstractListingState.java#L27)):*\n> ```Java\n>  *\n>  */\n> public class AbstractListingState extends AbstractFieldState {\n> ```\n\nListings are not fields, this mess needs to be cleaned up (congrats)\n\n---\n\n*[shared/src/main/java/com/vaadin/shared/ui/abstractmultiselect/AbstractMultiSelectState.java, line 27 at r1](https://reviewable.io:443/reviews/vaadin/framework/98#-KYO4yAK9vG0n3JswqxN:-KYO4yAKA7W_B0IFk5zH:b-u2dade) ([raw file](https://github.com/vaadin/framework/blob/138d7cac6a10f32f61a4fcee27d7b2eca02f7494/shared/src/main/java/com/vaadin/shared/ui/abstractmultiselect/AbstractMultiSelectState.java#L27)):*\n> ```Java\n>  *\n>  */\n> public class AbstractMultiSelectState extends AbstractListingState {\n> ```\n\nSame as with abstractsingleselect, this needs to extend abstractslisting state and provide readonly & required\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/vaadin/framework/98)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"pleku"}}