{"action":"created","issue":{"url":"https://api.github.com/repos/auth0/cosmos/issues/718","repository_url":"https://api.github.com/repos/auth0/cosmos","labels_url":"https://api.github.com/repos/auth0/cosmos/issues/718/labels{/name}","comments_url":"https://api.github.com/repos/auth0/cosmos/issues/718/comments","events_url":"https://api.github.com/repos/auth0/cosmos/issues/718/events","html_url":"https://github.com/auth0/cosmos/pull/718","id":343952184,"node_id":"MDExOlB1bGxSZXF1ZXN0MjAzNDYwNTgy","number":718,"title":"Form actions pass props through","user":{"login":"siddharthkp","id":1863771,"node_id":"MDQ6VXNlcjE4NjM3NzE=","avatar_url":"https://avatars0.githubusercontent.com/u/1863771?v=4","gravatar_id":"","url":"https://api.github.com/users/siddharthkp","html_url":"https://github.com/siddharthkp","followers_url":"https://api.github.com/users/siddharthkp/followers","following_url":"https://api.github.com/users/siddharthkp/following{/other_user}","gists_url":"https://api.github.com/users/siddharthkp/gists{/gist_id}","starred_url":"https://api.github.com/users/siddharthkp/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/siddharthkp/subscriptions","organizations_url":"https://api.github.com/users/siddharthkp/orgs","repos_url":"https://api.github.com/users/siddharthkp/repos","events_url":"https://api.github.com/users/siddharthkp/events{/privacy}","received_events_url":"https://api.github.com/users/siddharthkp/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2018-07-24T09:16:54Z","updated_at":"2018-07-25T19:56:39Z","closed_at":null,"author_association":"COLLABORATOR","pull_request":{"url":"https://api.github.com/repos/auth0/cosmos/pulls/718","html_url":"https://github.com/auth0/cosmos/pull/718","diff_url":"https://github.com/auth0/cosmos/pull/718.diff","patch_url":"https://github.com/auth0/cosmos/pull/718.patch"},"body":"Fixes https://github.com/auth0/cosmos/issues/665\r\n\r\nAdded additional `props` prop that is forwarded to the `Button` component\r\n\r\n```jsx  \r\n<Form.Actions\r\n  primaryAction={{ label: 'Save Changes', props: { disabled: true }, handler: () => {} }}\r\n/>\r\n```\r\n\r\nDoes this API look a little wonky? Should we use a more explicit name like `extraProps` or `buttonProps`?\r\n\r\n&nbsp;\r\n\r\nDemo: [form-actions#passing-button-props](https://auth0-cosmos-form-actions-pass-props-through.now.sh/#/component/form-actions#passing-button-props)\r\n\r\n#### misuse \r\n\r\nThere is little scope for misuse here, the `appearance` property comes after the spread props which means it will override it even if someone tries to pass `appearance=\"primary\"` in a destructive action\r\n\r\n```jsx\r\n<Button {...destructiveAction.props} appearance=\"destructive\" icon={destructiveAction.icon}>\r\n  {destructiveAction.label}\r\n</Button>\r\n```\r\n\r\nThere is however room for indirection. These 2 blocks have the same effect:\r\n\r\n```jsx\r\nprimaryAction={{ label: 'Save Changes', icon: 'home', props: { disabled: true }, handler: () => {} }}\r\n\r\nprimaryAction={{ label: 'Save Changes', props: { icon: 'home', disabled: true }, handler: () => {} }}\r\n```\r\n\r\nWe pass the `icon` prop transparently to `Button` right now, but could be troublesome in the future\r\n\r\n#### alternate syntax\r\n\r\nWe can offer the full control which is open to misuse but it supports all props directly because it is more composable\r\n\r\n```jsx  \r\n<Form.Actions>\r\n  <Button appearance=\"primary\" disabled={true} >Save Changes</Button>\r\n  <Button appearance=\"primary\">Two save buttons</Button>\r\n</Form.Actions>\r\n```"},"comment":{"url":"https://api.github.com/repos/auth0/cosmos/issues/comments/407875773","html_url":"https://github.com/auth0/cosmos/pull/718#issuecomment-407875773","issue_url":"https://api.github.com/repos/auth0/cosmos/issues/718","id":407875773,"node_id":"MDEyOklzc3VlQ29tbWVudDQwNzg3NTc3Mw==","user":{"login":"landitus","id":239215,"node_id":"MDQ6VXNlcjIzOTIxNQ==","avatar_url":"https://avatars0.githubusercontent.com/u/239215?v=4","gravatar_id":"","url":"https://api.github.com/users/landitus","html_url":"https://github.com/landitus","followers_url":"https://api.github.com/users/landitus/followers","following_url":"https://api.github.com/users/landitus/following{/other_user}","gists_url":"https://api.github.com/users/landitus/gists{/gist_id}","starred_url":"https://api.github.com/users/landitus/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/landitus/subscriptions","organizations_url":"https://api.github.com/users/landitus/orgs","repos_url":"https://api.github.com/users/landitus/repos","events_url":"https://api.github.com/users/landitus/events{/privacy}","received_events_url":"https://api.github.com/users/landitus/received_events","type":"User","site_admin":false},"created_at":"2018-07-25T19:56:39Z","updated_at":"2018-07-25T19:56:39Z","author_association":"COLLABORATOR","body":"> I don't really like the first option, using props. IMO, if we want to forward props directly, I think we should just forward the contents of the action object itself on to Button.\r\n\r\nI agree ! I think it would be best to pass all props directly to the `primaryAction` like:\r\n\r\n```\r\n<Form>\r\n...\r\n<Form.Actions\r\n  primaryAction={{ label: 'Save Changes', disabled: true, type: 'submit', loading: false, onClick: () => {} }}\r\n secondaryAction={{ label: 'Cancel', disabled: true, type: 'button', loading: false, onClick: () => {} }}\r\n/>\r\n</Form>\r\n```\r\n\r\nWe could filter setting the `appearance` prop, but ultimately you should be able to pass all properties for the button (or a subset). I'm not so much concerned about a misuse, as we are providing also documented guidelines on how this actions should be presented. It's more about saving time baking in decisions like spacing, button appearance while getting more control over the presentation.\r\n\r\nOne step further would be to create a template for this button shape so that it's easy to set and maintain. I'm looking at Polaris for this, and looks good: \r\n\r\n- https://polaris.shopify.com/components/structure/page#navigation\r\n- https://polaris.shopify.com/components/structure/page-actions#navigation\r\n\r\nIn the end, it's about the Open/close debate, which I see valid points for both. We are using the closed component method selectively on the compound components to get more control over the styling, while always supplying the internal parts if you wish to extend it or [DIY.]\r\n\r\nJust to make a fair comparison with the first option, the composable one could be:\r\n```\r\n<Form>\r\n...\r\n<ButtonGroup>\r\n  <Button appearance='primary' disabled='true' loading='false' onClick=''>Save</Button>\r\n  <Button disabled='true' loading='false' onClick=''>Cancel</Button>\r\n</ButtonGroup> \r\n</Form> \r\n```\r\nwdyt?"}}
{"actor":{"display_login":"landitus"}}